Skip to content

Commit f004564

Browse files
committed
Fix shortcut methods ignoring handler errors
Fixes #2196
1 parent bf84d36 commit f004564

File tree

2 files changed

+114
-7
lines changed

2 files changed

+114
-7
lines changed

source/as-promise/index.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,23 +220,27 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
220220
return promise;
221221
};
222222

223-
const shortcut = <T>(responseType: Options['responseType']): CancelableRequest<T> => {
223+
const shortcut = <T>(promiseToAwait: CancelableRequest<any>, responseType: Options['responseType']): CancelableRequest<T> => {
224224
const newPromise = (async () => {
225225
// Wait until downloading has ended
226-
await promise;
226+
await promiseToAwait;
227227

228228
const {options} = globalResponse.request;
229229

230230
return parseBody(globalResponse, responseType, options.parseJson, options.encoding);
231231
})();
232232

233233
// eslint-disable-next-line @typescript-eslint/no-floating-promises
234-
Object.defineProperties(newPromise, Object.getOwnPropertyDescriptors(promise));
234+
Object.defineProperties(newPromise, Object.getOwnPropertyDescriptors(promiseToAwait));
235235

236236
return newPromise as CancelableRequest<T>;
237237
};
238238

239-
promise.json = () => {
239+
// Note: These use `function` syntax (not arrows) to access `this` context.
240+
// When custom handlers wrap the promise to transform errors, these methods
241+
// are copied to the handler's promise. Using `this` ensures we await the
242+
// handler's wrapped promise, not the original, so errors propagate correctly.
243+
promise.json = function (this: CancelableRequest<any>) {
240244
if (globalRequest.options) {
241245
const {headers} = globalRequest.options;
242246

@@ -245,11 +249,16 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
245249
}
246250
}
247251

248-
return shortcut('json');
252+
return shortcut(this, 'json');
249253
};
250254

251-
promise.buffer = () => shortcut('buffer');
252-
promise.text = () => shortcut('text');
255+
promise.buffer = function (this: CancelableRequest<any>) {
256+
return shortcut(this, 'buffer');
257+
};
258+
259+
promise.text = function (this: CancelableRequest<any>) {
260+
return shortcut(this, 'text');
261+
};
253262

254263
return promise;
255264
}

test/hooks.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2128,3 +2128,101 @@ test('beforeRetry handles stream to Buffer conversion', withServer, async (t, se
21282128
t.is(response.body, 'Got: buffer-data');
21292129
t.is(requestCount, 2);
21302130
});
2131+
2132+
test('handler error is properly thrown in .json()', withServer, async (t, _server, got) => {
2133+
const customError = new Error('Custom handler error');
2134+
const instance = got.extend({
2135+
handlers: [
2136+
(options, next) => (async () => {
2137+
try {
2138+
return await next(options);
2139+
} catch {
2140+
throw customError;
2141+
}
2142+
})(),
2143+
],
2144+
});
2145+
2146+
await t.throwsAsync(instance('').json(), {message: 'Custom handler error'});
2147+
});
2148+
2149+
test('handler error is properly thrown in .text()', withServer, async (t, _server, got) => {
2150+
const customError = new Error('Custom handler error for text');
2151+
const instance = got.extend({
2152+
handlers: [
2153+
(options, next) => (async () => {
2154+
try {
2155+
return await next(options);
2156+
} catch {
2157+
throw customError;
2158+
}
2159+
})(),
2160+
],
2161+
});
2162+
2163+
await t.throwsAsync(instance('').text(), {message: 'Custom handler error for text'});
2164+
});
2165+
2166+
test('handler error is properly thrown in .buffer()', withServer, async (t, _server, got) => {
2167+
const customError = new Error('Custom handler error for buffer');
2168+
const instance = got.extend({
2169+
handlers: [
2170+
(options, next) => (async () => {
2171+
try {
2172+
return await next(options);
2173+
} catch {
2174+
throw customError;
2175+
}
2176+
})(),
2177+
],
2178+
});
2179+
2180+
await t.throwsAsync(instance('').buffer(), {message: 'Custom handler error for buffer'});
2181+
});
2182+
2183+
test('handler throwing on successful response works with .json()', withServer, async (t, server, got) => {
2184+
server.get('/', (_request, response) => {
2185+
response.setHeader('content-type', 'application/json');
2186+
response.end('{"success": true}');
2187+
});
2188+
2189+
const customError = new Error('Handler rejected success');
2190+
const instance = got.extend({
2191+
handlers: [
2192+
(options, next) => (async () => {
2193+
await next(options);
2194+
throw customError;
2195+
})(),
2196+
],
2197+
});
2198+
2199+
await t.throwsAsync(instance('').json(), {message: 'Handler rejected success'});
2200+
});
2201+
2202+
test('multiple handlers with error transformation work with .json()', withServer, async (t, _server, got) => {
2203+
const instance = got.extend({
2204+
handlers: [
2205+
// First handler: catches and wraps error
2206+
(options, next) => (async () => {
2207+
try {
2208+
return await next(options);
2209+
} catch (error: any) {
2210+
const wrappedError = new Error(`Handler 1: ${error.message}`);
2211+
throw wrappedError;
2212+
}
2213+
})(),
2214+
// Second handler: catches and wraps error again
2215+
(options, next) => (async () => {
2216+
try {
2217+
return await next(options);
2218+
} catch (error: any) {
2219+
const wrappedError = new Error(`Handler 2: ${error.message}`);
2220+
throw wrappedError;
2221+
}
2222+
})(),
2223+
],
2224+
});
2225+
2226+
// Should get error from first handler (outermost)
2227+
await t.throwsAsync(instance('').json(), {message: /Handler 1: Handler 2:/});
2228+
});

0 commit comments

Comments
 (0)