diff --git a/documentation/3-streams.md b/documentation/3-streams.md index 9070e43da..5fe237cb9 100644 --- a/documentation/3-streams.md +++ b/documentation/3-streams.md @@ -239,6 +239,11 @@ To enable retrying when using streams, a retry handler must be attached. When this event is emitted, you should reset the stream you were writing to and prepare the body again. +**Note:** +> - [`HTTPError`s](./8-errors.md#httperror) cannot be retried if [`options.throwHttpErrors`](./2-options.md#throwhttperrors) is `false`. +> This is because stream data is saved to `error.response.body` and streams can be read only once. +> - For the Promise API, there is no such limitation. + #### `retryCount` **Type: `number`** diff --git a/source/core/index.ts b/source/core/index.ts index 079d349ac..876ff9d08 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -793,6 +793,10 @@ export default class Request extends Duplex implements RequestEvents { return; } + // `HTTPError`s always have `error.response.body` defined. + // Therefore we cannot retry if `options.throwHttpErrors` is false. + // On the last retry, if `options.throwHttpErrors` is false, we would need to return the body, + // but that wouldn't be possible since the body would be already read in `error.response.body`. if (options.isStream && options.throwHttpErrors && !isResponseOk(typedResponse)) { this._beforeError(new HTTPError(typedResponse)); return; @@ -1144,9 +1148,15 @@ export default class Request extends Duplex implements RequestEvents { private async _error(error: RequestError): Promise { try { - for (const hook of this.options.hooks.beforeError) { - // eslint-disable-next-line no-await-in-loop - error = await hook(error); + if (error instanceof HTTPError && !this.options.throwHttpErrors) { + // This branch can be reached only when using the Promise API + // Skip calling the hooks on purpose. + // See https://github.com/sindresorhus/got/issues/2103 + } else { + for (const hook of this.options.hooks.beforeError) { + // eslint-disable-next-line no-await-in-loop + error = await hook(error); + } } } catch (error_: any) { error = new RequestError(error_.message, error_, this); diff --git a/test/hooks.ts b/test/hooks.ts index 39b41b59c..1b76fc656 100644 --- a/test/hooks.ts +++ b/test/hooks.ts @@ -1366,3 +1366,26 @@ test('does not throw on empty body when running afterResponse hooks', withServer }, })); }); + +test('does not call beforeError hooks on falsy throwHttpErrors', withServer, async (t, server, got) => { + server.get('/', (_request, response) => { + response.statusCode = 404; + response.end(); + }); + + let called = false; + + await got('', { + throwHttpErrors: false, + hooks: { + beforeError: [ + error => { + called = true; + return error; + }, + ], + }, + }); + + t.false(called); +});