Skip to content

Commit 1e49781

Browse files
committed
Fix race condition causing retry after promise settles
Fixes #1489
1 parent 91cdc48 commit 1e49781

File tree

2 files changed

+68
-0
lines changed

2 files changed

+68
-0
lines changed

source/as-promise/index.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
2929
let globalResponse: Response;
3030
let normalizedOptions: Options;
3131
const emitter = new EventEmitter();
32+
let promiseSettled = false;
3233

3334
const promise = new PCancelable<T>((resolve, reject, onCancel) => {
3435
onCancel(() => {
@@ -37,6 +38,7 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
3738

3839
onCancel.shouldReject = false;
3940
onCancel(() => {
41+
promiseSettled = true;
4042
reject(new CancelError(globalRequest));
4143
});
4244

@@ -123,6 +125,7 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
123125
}
124126

125127
request.destroy();
128+
promiseSettled = true;
126129
resolve(request.options.resolveBodyOnly ? response.body as T : response as unknown as T);
127130
});
128131

@@ -148,6 +151,7 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
148151

149152
handledFinalError = true;
150153

154+
promiseSettled = true;
151155
const {options} = request;
152156

153157
if (error instanceof HTTPError && !options.throwHttpErrors) {
@@ -172,6 +176,14 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
172176
request.once('retry', (newRetryCount: number, error: RequestError) => {
173177
firstRequest = undefined;
174178

179+
// If promise already settled, don't retry
180+
// This prevents the race condition in #1489 where a late error
181+
// (e.g., ECONNRESET after successful response) triggers retry
182+
// after the promise has already resolved/rejected
183+
if (promiseSettled) {
184+
return;
185+
}
186+
175187
const newBody = request.options.body;
176188

177189
if (previousBody === newBody && is.nodeStream(newBody)) {

test/retry.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,3 +838,59 @@ test('does not retry on stream errors when not in errorCodes', async t => {
838838
t.is(attemptCount, 1);
839839
t.is(error?.code, 'ETEST');
840840
});
841+
842+
test('does not retry after promise settles (issue #1489)', async t => {
843+
let retryTriggered = false;
844+
845+
const response = await got('https://example.com', {
846+
retry: {limit: 2, errorCodes: ['ECONNRESET']},
847+
hooks: {
848+
beforeRetry: [() => {
849+
retryTriggered = true;
850+
}],
851+
},
852+
request() {
853+
const emitter = new EventEmitter() as http.ClientRequest;
854+
emitter.abort = () => {};
855+
// @ts-expect-error Imitating a stream
856+
emitter.end = () => {};
857+
emitter.destroyed = false;
858+
// @ts-expect-error Imitating a stream
859+
emitter.destroy = () => {
860+
emitter.destroyed = true;
861+
};
862+
863+
emitter.write = () => true;
864+
// @ts-expect-error Imitating a stream
865+
emitter.writable = true;
866+
// @ts-expect-error Imitating a stream
867+
emitter.writableEnded = false;
868+
869+
setTimeout(() => {
870+
const incomingMessage = new PassThroughStream() as unknown as http.IncomingMessage;
871+
incomingMessage.statusCode = 200;
872+
incomingMessage.headers = {};
873+
874+
emitter.emit('response', incomingMessage);
875+
876+
setImmediate(() => {
877+
// @ts-expect-error PassThrough method
878+
incomingMessage.end('ok');
879+
});
880+
881+
// Late error after response - should NOT trigger retry
882+
setTimeout(() => {
883+
const error = new Error('read ECONNRESET');
884+
(error as NodeJS.ErrnoException).code = 'ECONNRESET';
885+
emitter.emit('error', error);
886+
}, 10);
887+
});
888+
889+
return emitter;
890+
},
891+
throwHttpErrors: false,
892+
});
893+
894+
t.is(response.statusCode, 200);
895+
t.false(retryTriggered);
896+
});

0 commit comments

Comments
 (0)