Got: Custom errors thrown in afterResponse are suppressed

[simontabor]

Describe the bug

If my afterResponse hook throws a custom error, I'm not expecting it to be rewritten into a Got Request Error

Actual behavior

Error is rewritten as a RequestError, with no reference to the original error

RequestError: Bad request signature

Expected behavior

Original error is thrown

HmacError: Bad request signature

Code to reproduce

const got = require('got');
const assert = require('assert');

class HmacError extends Error {}

got({
  url: 'https://not-here.com',
  hooks: {
    afterResponse: [
      () => {
        throw new HmacError('Bad request signature');
      },
    ],
  },
}).catch(err => {
  assert.equal(err.name, 'HmacError');
});

This will throw -

(node:73559) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: 'RequestError' == 'HmacError'

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

[szmarczak]

This behavior is intended as the Got errors provide useful properties for debugging. Instead of overwriting the error, we could write into it.

[simontabor]

@szmarczak that makes sense... I'd rather we treat the errors as immutable (otherwise you can end up in a right old mess with Typescript!)

How about we keep a reference to the originalError within the RequestError?

[simontabor]

Just also noticed from the error that only the timings are attached to the error - how can I access the underlying request or response when my afterResponse handler throws an error? Might be worth attaching request and response wherever possible too (potentially non-enumerable), I'd like to see what failed request was rather than just the timings

[szmarczak]

Actually if it's missing request and response properties at this point then it's a bug... It should have that already.

[simontabor]

You're right, sorry about the last comment, didn't even check - it's there but non-enumerable already so didn't show up in my console.log

[szmarczak]

It's non-enumerable so if you use Sentry or any other error-catching API you won't get bloated data. Although I think we could fix that. See #1067

[szmarczak]

Closing in favor of #1353