Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't call beforeError hooks with HTTPError if !throwHttpErrors #2104

Merged
merged 3 commits into from Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 13 additions & 3 deletions source/core/index.ts
Expand Up @@ -793,6 +793,10 @@ export default class Request extends Duplex implements RequestEvents<Request> {
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`.
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
if (options.isStream && options.throwHttpErrors && !isResponseOk(typedResponse)) {
this._beforeError(new HTTPError(typedResponse));
return;
Expand Down Expand Up @@ -1144,9 +1148,15 @@ export default class Request extends Duplex implements RequestEvents<Request> {

private async _error(error: RequestError): Promise<void> {
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);
Expand Down
23 changes: 23 additions & 0 deletions test/hooks.ts
Expand Up @@ -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);
});