Skip to content

Commit

Permalink
Fix the retry logic
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak committed Apr 15, 2020
1 parent b9a3417 commit 5d69522
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 14 deletions.
19 changes: 9 additions & 10 deletions source/as-promise/core.ts
Expand Up @@ -6,7 +6,7 @@ import {
Defaults,
ResponseType
} from './types';
import Request, {knownHookEvents, RequestError, HTTPError, Method} from '../core';
import Request, {knownHookEvents, RequestError, Method} from '../core';

if (!knownHookEvents.includes('beforeRetry' as any)) {
knownHookEvents.push('beforeRetry' as any, 'afterResponse' as any);
Expand Down Expand Up @@ -126,23 +126,22 @@ export default class PromisableRequest extends Request {
return mergedOptions!;
}

async _beforeError(error: RequestError): Promise<void> {
const isHTTPError = error instanceof HTTPError;
async _beforeError(error: Error): Promise<void> {
if (!(error instanceof RequestError)) {
error = new RequestError(error.message, error, this.options, this);
}

try {
for (const hook of this.options.hooks.beforeError) {
// eslint-disable-next-line no-await-in-loop
error = await hook(error);
error = await hook(error as RequestError);
}
} catch (error_) {
this.destroy(error_);
this.destroy(new RequestError(error_.message, error_, this.options, this));
return;
}

if (this._throwHttpErrors && !isHTTPError) {
this.destroy(error);
} else {
this.emit('error', error);
}
// Let the promise handle the error
this.emit('error', error);
}
}
4 changes: 4 additions & 0 deletions source/as-promise/index.ts
Expand Up @@ -34,6 +34,10 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
}

// Support retries
// `options.throwHttpErrors` needs to be always true,
// so the HTTP errors are caught and the request is retried.
// The error is **eventaully** thrown
// if the user value `options._throwHttpErrors` is true.
const {throwHttpErrors} = options;
if (!throwHttpErrors) {
options.throwHttpErrors = true;
Expand Down
9 changes: 5 additions & 4 deletions source/core/index.ts
Expand Up @@ -38,6 +38,7 @@ const kUnproxyEvents = Symbol('unproxyEvents');
const kIsFromCache = Symbol('isFromCache');
const kCancelTimeouts = Symbol('cancelTimeouts');
const kStartedReading = Symbol('startedReading');
const kStopReading = Symbol('stopReading');
export const kIsNormalizedAlready = Symbol('isNormalizedAlready');

const supportsBrotli = is.string((process.versions as any).brotli);
Expand Down Expand Up @@ -466,6 +467,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
declare _cannotHaveBody: boolean;
[kDownloadedSize]: number;
[kUploadedSize]: number;
[kStopReading]: boolean;
[kBodySize]?: number;
[kServerResponsesPiped]: Set<ServerResponse>;
[kIsFromCache]?: boolean;
Expand All @@ -481,7 +483,6 @@ export default class Request extends Duplex implements RequestEvents<Request> {
declare requestUrl: string;
finalized: boolean;
redirects: string[];
errored: boolean;

constructor(url: string | URL, options: Options = {}, defaults?: Defaults) {
super({
Expand All @@ -494,7 +495,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
this.finalized = false;
this[kServerResponsesPiped] = new Set<ServerResponse>();
this.redirects = [];
this.errored = false;
this[kStopReading] = false;

// TODO: Remove this when targeting Node.js >= 12
this._progressCallbacks = [];
Expand Down Expand Up @@ -1299,7 +1300,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
}

async _beforeError(error: Error): Promise<void> {
this.errored = true;
this[kStopReading] = true;

if (!(error instanceof RequestError)) {
error = new RequestError(error.message, error, this.options, this);
Expand Down Expand Up @@ -1328,7 +1329,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
}

_read(): void {
if (kResponse in this && !this.errored) {
if (kResponse in this && !this[kStopReading]) {
let data;

while ((data = this[kResponse]!.read()) !== null) {
Expand Down

0 comments on commit 5d69522

Please sign in to comment.