Skip to content

Commit af0b147

Browse files
szmarczakszmarczak
committed
Add a request property to errors
1 parent c56c33a commit af0b147

File tree

5 files changed

+87
-86
lines changed

5 files changed

+87
-86
lines changed

‎source/as-promise/core.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export default class PromisableRequest extends Request {
140140

141141
async _beforeError(error: Error): Promise<void> {
142142
if (!(error instanceof RequestError)) {
143-
error = new RequestError(error.message, error, this.options, this);
143+
error = new RequestError(error.message, error, this);
144144
}
145145

146146
try {
@@ -149,7 +149,7 @@ export default class PromisableRequest extends Request {
149149
error = await hook(error as RequestError);
150150
}
151151
} catch (error_) {
152-
this.destroy(new RequestError(error_.message, error_, this.options, this));
152+
this.destroy(new RequestError(error_.message, error_, this));
153153
return;
154154
}
155155

‎source/as-promise/index.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
6666

6767
response.rawBody = rawBody;
6868
} catch (error) {
69-
request._beforeError(new ReadError(error, options, response));
69+
request._beforeError(new ReadError(error, request));
7070
return;
7171
}
7272

@@ -124,7 +124,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
124124
}
125125

126126
if (throwHttpErrors && !isOk()) {
127-
reject(new HTTPError(response, options));
127+
reject(new HTTPError(response));
128128
return;
129129
}
130130

@@ -163,7 +163,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
163163
// Don't emit the `response` event
164164
request.destroy();
165165

166-
reject(new RequestError(error_.message, error, request.options));
166+
reject(new RequestError(error_.message, error, request));
167167
return;
168168
}
169169

@@ -183,7 +183,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
183183
// Don't emit the `response` event
184184
request.destroy();
185185

186-
reject(new RequestError(error_.message, error, request.options));
186+
reject(new RequestError(error_.message, error, request));
187187
return;
188188
}
189189

‎source/as-promise/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export class ParseError extends RequestError {
112112
constructor(error: Error, response: Response) {
113113
const {options} = response.request;
114114

115-
super(`${error.message} in "${options.url.toString()}"`, error, options);
115+
super(`${error.message} in "${options.url.toString()}"`, error, response.request);
116116
this.name = 'ParseError';
117117

118118
Object.defineProperty(this, 'response', {

‎source/core/index.ts

Lines changed: 68 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -254,32 +254,6 @@ function isClientRequest(clientRequest: unknown): clientRequest is ClientRequest
254254

255255
const cacheableStore = new WeakableMap<string | CacheableRequest.StorageAdapter, CacheableRequestFn>();
256256

257-
const cacheFn = async (url: URL, options: RequestOptions): Promise<ClientRequest | ResponseLike> => new Promise<ClientRequest | ResponseLike>((resolve, reject) => {
258-
// TODO: Remove `utils/url-to-options.ts` when `cacheable-request` is fixed
259-
Object.assign(options, urlToOptions(url));
260-
261-
// `http-cache-semantics` checks this
262-
delete (options as unknown as NormalizedOptions).url;
263-
264-
// This is ugly
265-
const cacheRequest = cacheableStore.get((options as any).cache)!(options, resolve as any);
266-
267-
// Restore options
268-
(options as unknown as NormalizedOptions).url = url;
269-
270-
cacheRequest.once('error', (error: Error) => {
271-
if (error instanceof CacheableRequest.RequestError) {
272-
// TODO: `options` should be `normalizedOptions`
273-
reject(new RequestError(error.message, error, options as unknown as NormalizedOptions));
274-
return;
275-
}
276-
277-
// TODO: `options` should be `normalizedOptions`
278-
reject(new CacheError(error, options as unknown as NormalizedOptions));
279-
});
280-
cacheRequest.once('request', resolve);
281-
});
282-
283257
const waitForOpenFile = async (file: ReadStream): Promise<void> => new Promise((resolve, reject) => {
284258
const onError = (error: Error): void => {
285259
reject(error);
@@ -337,39 +311,36 @@ export class RequestError extends Error {
337311
readonly request?: Request;
338312
readonly timings?: Timings;
339313

340-
constructor(message: string, error: Partial<Error & {code?: string}>, options: NormalizedOptions, requestOrResponse?: Request | Response) {
314+
constructor(message: string, error: Partial<Error & {code?: string}>, self: Request | NormalizedOptions) {
341315
super(message);
342316
Error.captureStackTrace(this, this.constructor);
343317

344318
this.name = 'RequestError';
345319
this.code = error.code;
346320

347-
Object.defineProperty(this, 'options', {
348-
// This fails because of TS 3.7.2 useDefineForClassFields
349-
// Ref: https://github.com/microsoft/TypeScript/issues/34972
350-
enumerable: false,
351-
value: options
352-
});
353-
354-
if (requestOrResponse instanceof IncomingMessage) {
355-
Object.defineProperty(this, 'response', {
321+
if (self instanceof Request) {
322+
Object.defineProperty(this, 'request', {
356323
enumerable: false,
357-
value: requestOrResponse
324+
value: self
358325
});
359326

360-
Object.defineProperty(this, 'request', {
327+
Object.defineProperty(this, 'response', {
361328
enumerable: false,
362-
value: requestOrResponse.request
329+
value: self[kResponse]
363330
});
364-
} else if (requestOrResponse instanceof Request) {
365-
Object.defineProperty(this, 'request', {
331+
332+
Object.defineProperty(this, 'options', {
333+
// This fails because of TS 3.7.2 useDefineForClassFields
334+
// Ref: https://github.com/microsoft/TypeScript/issues/34972
366335
enumerable: false,
367-
value: requestOrResponse
336+
value: self.options
368337
});
369-
370-
Object.defineProperty(this, 'response', {
338+
} else {
339+
Object.defineProperty(this, 'options', {
340+
// This fails because of TS 3.7.2 useDefineForClassFields
341+
// Ref: https://github.com/microsoft/TypeScript/issues/34972
371342
enumerable: false,
372-
value: requestOrResponse[kResponse]
343+
value: self
373344
});
374345
}
375346

@@ -394,41 +365,31 @@ export class RequestError extends Error {
394365
export class MaxRedirectsError extends RequestError {
395366
declare readonly response: Response;
396367

397-
constructor(response: Response, maxRedirects: number, options: NormalizedOptions) {
398-
super(`Redirected ${maxRedirects} times. Aborting.`, {}, options);
368+
constructor(request: Request) {
369+
super(`Redirected ${request.options.maxRedirects} times. Aborting.`, {}, request);
399370
this.name = 'MaxRedirectsError';
400-
401-
Object.defineProperty(this, 'response', {
402-
enumerable: false,
403-
value: response
404-
});
405371
}
406372
}
407373

408374
export class HTTPError extends RequestError {
409375
declare readonly response: Response;
410376

411-
constructor(response: Response, options: NormalizedOptions) {
412-
super(`Response code ${response.statusCode} (${response.statusMessage!})`, {}, options);
377+
constructor(response: Response) {
378+
super(`Response code ${response.statusCode} (${response.statusMessage!})`, {}, response.request);
413379
this.name = 'HTTPError';
414-
415-
Object.defineProperty(this, 'response', {
416-
enumerable: false,
417-
value: response
418-
});
419380
}
420381
}
421382

422383
export class CacheError extends RequestError {
423-
constructor(error: Error, options: NormalizedOptions) {
424-
super(error.message, error, options);
384+
constructor(error: Error, request: Request) {
385+
super(error.message, error, request);
425386
this.name = 'CacheError';
426387
}
427388
}
428389

429390
export class UploadError extends RequestError {
430-
constructor(error: Error, options: NormalizedOptions, request: Request) {
431-
super(error.message, error, options, request);
391+
constructor(error: Error, request: Request) {
392+
super(error.message, error, request);
432393
this.name = 'UploadError';
433394
}
434395
}
@@ -437,17 +398,17 @@ export class TimeoutError extends RequestError {
437398
readonly timings: Timings;
438399
readonly event: string;
439400

440-
constructor(error: TimedOutTimeoutError, timings: Timings, options: NormalizedOptions) {
441-
super(error.message, error, options);
401+
constructor(error: TimedOutTimeoutError, timings: Timings, request: Request) {
402+
super(error.message, error, request);
442403
this.name = 'TimeoutError';
443404
this.event = error.event;
444405
this.timings = timings;
445406
}
446407
}
447408

448409
export class ReadError extends RequestError {
449-
constructor(error: Error, options: NormalizedOptions, response: Response) {
450-
super(error.message, error, options, response);
410+
constructor(error: Error, request: Request) {
411+
super(error.message, error, request);
451412
this.name = 'ReadError';
452413
}
453414
}
@@ -982,7 +943,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
982943
});
983944

984945
response.on('error', (error: Error) => {
985-
this._beforeError(new ReadError(error, options, response as Response));
946+
this._beforeError(new ReadError(error, this));
986947
});
987948

988949
response.once('aborted', () => {
@@ -993,7 +954,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
993954
this._beforeError(new ReadError({
994955
name: 'Error',
995956
message: 'The server aborted the pending request'
996-
}, options, response as Response));
957+
}, this));
997958
});
998959

999960
this.emit('downloadProgress', this.downloadProgress);
@@ -1048,7 +1009,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
10481009
}
10491010

10501011
if (this.redirects.length >= options.maxRedirects) {
1051-
this._beforeError(new MaxRedirectsError(typedResponse, options.maxRedirects, options));
1012+
this._beforeError(new MaxRedirectsError(this));
10521013
return;
10531014
}
10541015

@@ -1097,7 +1058,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
10971058
const limitStatusCode = options.followRedirect ? 299 : 399;
10981059
const isOk = (statusCode >= 200 && statusCode <= limitStatusCode) || statusCode === 304;
10991060
if (options.throwHttpErrors && !isOk) {
1100-
await this._beforeError(new HTTPError(typedResponse, options));
1061+
await this._beforeError(new HTTPError(typedResponse));
11011062

11021063
if (this.destroyed) {
11031064
return;
@@ -1157,9 +1118,9 @@ export default class Request extends Duplex implements RequestEvents<Request> {
11571118

11581119
request.once('error', (error: Error) => {
11591120
if (error instanceof TimedOutTimeoutError) {
1160-
error = new TimeoutError(error, this.timings!, options);
1121+
error = new TimeoutError(error, this.timings!, this);
11611122
} else {
1162-
error = new RequestError(error.message, error, options, this);
1123+
error = new RequestError(error.message, error, this);
11631124
}
11641125

11651126
this._beforeError(error as RequestError);
@@ -1176,7 +1137,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
11761137
if (is.nodeStream(options.body)) {
11771138
options.body.pipe(currentRequest);
11781139
options.body.once('error', (error: NodeJS.ErrnoException) => {
1179-
this._beforeError(new UploadError(error, options, this));
1140+
this._beforeError(new UploadError(error, this));
11801141
});
11811142

11821143
options.body.once('end', () => {
@@ -1200,6 +1161,34 @@ export default class Request extends Duplex implements RequestEvents<Request> {
12001161
this.emit('request', request);
12011162
}
12021163

1164+
async _createCacheableRequest(url: URL, options: RequestOptions): Promise<ClientRequest | ResponseLike> {
1165+
return new Promise<ClientRequest | ResponseLike>((resolve, reject) => {
1166+
// TODO: Remove `utils/url-to-options.ts` when `cacheable-request` is fixed
1167+
Object.assign(options, urlToOptions(url));
1168+
1169+
// `http-cache-semantics` checks this
1170+
delete (options as unknown as NormalizedOptions).url;
1171+
1172+
// This is ugly
1173+
const cacheRequest = cacheableStore.get((options as any).cache)!(options, resolve as any);
1174+
1175+
// Restore options
1176+
(options as unknown as NormalizedOptions).url = url;
1177+
1178+
cacheRequest.once('error', (error: Error) => {
1179+
if (error instanceof CacheableRequest.RequestError) {
1180+
// TODO: `options` should be `normalizedOptions`
1181+
reject(new RequestError(error.message, error, this));
1182+
return;
1183+
}
1184+
1185+
// TODO: `options` should be `normalizedOptions`
1186+
reject(new CacheError(error, this));
1187+
});
1188+
cacheRequest.once('request', resolve);
1189+
});
1190+
}
1191+
12031192
async _makeRequest(): Promise<void> {
12041193
const {options} = this;
12051194
const {url, headers, request, agent, timeout} = options;
@@ -1266,7 +1255,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
12661255
}
12671256

12681257
const realFn = options.request ?? fallbackFn;
1269-
const fn = options.cache ? cacheFn : realFn;
1258+
const fn = options.cache ? this._createCacheableRequest.bind(this) : realFn;
12701259

12711260
if (agent && !options.http2) {
12721261
(options as unknown as RequestOptions).agent = agent[isHttps ? 'https' : 'http'];
@@ -1310,15 +1299,15 @@ export default class Request extends Duplex implements RequestEvents<Request> {
13101299
throw error;
13111300
}
13121301

1313-
throw new RequestError(error.message, error, options, this);
1302+
throw new RequestError(error.message, error, this);
13141303
}
13151304
}
13161305

13171306
async _beforeError(error: Error): Promise<void> {
13181307
this[kStopReading] = true;
13191308

13201309
if (!(error instanceof RequestError)) {
1321-
error = new RequestError(error.message, error, this.options, this);
1310+
error = new RequestError(error.message, error, this);
13221311
}
13231312

13241313
try {
@@ -1338,7 +1327,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
13381327
error = await hook(error as RequestError);
13391328
}
13401329
} catch (error_) {
1341-
error = new RequestError(error_.message, error_, this.options, this);
1330+
error = new RequestError(error_.message, error_, this);
13421331
}
13431332

13441333
this.destroy(error);
@@ -1451,7 +1440,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
14511440
}
14521441

14531442
if (error !== null && !is.undefined(error) && !(error instanceof RequestError)) {
1454-
error = new RequestError(error.message, error, this.options, this);
1443+
error = new RequestError(error.message, error, this);
14551444
}
14561445

14571446
callback(error);

‎test/error.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,18 @@ test('normalization errors using convenience methods', async t => {
192192
await t.throwsAsync(got(url).json().text().buffer(), {message: `Invalid URL: ${url}`});
193193
});
194194

195+
test('errors can have request property', withServer, async (t, server, got) => {
196+
server.get('/', (_request, response) => {
197+
response.statusCode = 404;
198+
response.end();
199+
});
200+
201+
const error = await t.throwsAsync<HTTPError>(got(''));
202+
203+
t.truthy(error.response);
204+
t.truthy(error.request!.downloadProgress);
205+
});
206+
195207
// Fails randomly on Node 10:
196208
// Blocked by https://github.com/istanbuljs/nyc/issues/619
197209
// eslint-disable-next-line ava/no-skip-test

0 commit comments

Comments
 (0)