Skip to content

Commit

Permalink
Add a request property to errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@szmarczak
szmarczak committed Apr 18, 2020
1 parent c56c33a commit af0b147
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 86 deletions.
4 changes: 2 additions & 2 deletions source/as-promise/core.ts
View file
Expand Up @@ -140,7 +140,7 @@ export default class PromisableRequest extends Request {

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

try {
Expand All @@ -149,7 +149,7 @@ export default class PromisableRequest extends Request {
error = await hook(error as RequestError);
}
} catch (error_) {
this.destroy(new RequestError(error_.message, error_, this.options, this));
this.destroy(new RequestError(error_.message, error_, this));
return;
}

Expand Down
8 changes: 4 additions & 4 deletions source/as-promise/index.ts
View file
Expand Up @@ -66,7 +66,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ

response.rawBody = rawBody;
} catch (error) {
request._beforeError(new ReadError(error, options, response));
request._beforeError(new ReadError(error, request));
return;
}

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

if (throwHttpErrors && !isOk()) {
reject(new HTTPError(response, options));
reject(new HTTPError(response));
return;
}

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

reject(new RequestError(error_.message, error, request.options));
reject(new RequestError(error_.message, error, request));
return;
}

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

reject(new RequestError(error_.message, error, request.options));
reject(new RequestError(error_.message, error, request));
return;
}

Expand Down
2 changes: 1 addition & 1 deletion source/as-promise/types.ts
View file
Expand Up @@ -112,7 +112,7 @@ export class ParseError extends RequestError {
constructor(error: Error, response: Response) {
const {options} = response.request;

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

Object.defineProperty(this, 'response', {
Expand Down
147 changes: 68 additions & 79 deletions source/core/index.ts
View file
Expand Up @@ -254,32 +254,6 @@ function isClientRequest(clientRequest: unknown): clientRequest is ClientRequest

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

const cacheFn = async (url: URL, options: RequestOptions): Promise<ClientRequest | ResponseLike> => new Promise<ClientRequest | ResponseLike>((resolve, reject) => {
// TODO: Remove `utils/url-to-options.ts` when `cacheable-request` is fixed
Object.assign(options, urlToOptions(url));

// `http-cache-semantics` checks this
delete (options as unknown as NormalizedOptions).url;

// This is ugly
const cacheRequest = cacheableStore.get((options as any).cache)!(options, resolve as any);

// Restore options
(options as unknown as NormalizedOptions).url = url;

cacheRequest.once('error', (error: Error) => {
if (error instanceof CacheableRequest.RequestError) {
// TODO: `options` should be `normalizedOptions`
reject(new RequestError(error.message, error, options as unknown as NormalizedOptions));
return;
}

// TODO: `options` should be `normalizedOptions`
reject(new CacheError(error, options as unknown as NormalizedOptions));
});
cacheRequest.once('request', resolve);
});

const waitForOpenFile = async (file: ReadStream): Promise<void> => new Promise((resolve, reject) => {
const onError = (error: Error): void => {
reject(error);
Expand Down Expand Up @@ -337,39 +311,36 @@ export class RequestError extends Error {
readonly request?: Request;
readonly timings?: Timings;

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

this.name = 'RequestError';
this.code = error.code;

Object.defineProperty(this, 'options', {
// This fails because of TS 3.7.2 useDefineForClassFields
// Ref: https://github.com/microsoft/TypeScript/issues/34972
enumerable: false,
value: options
});

if (requestOrResponse instanceof IncomingMessage) {
Object.defineProperty(this, 'response', {
if (self instanceof Request) {
Object.defineProperty(this, 'request', {
enumerable: false,
value: requestOrResponse
value: self
});

Object.defineProperty(this, 'request', {
Object.defineProperty(this, 'response', {
enumerable: false,
value: requestOrResponse.request
value: self[kResponse]
});
} else if (requestOrResponse instanceof Request) {
Object.defineProperty(this, 'request', {

Object.defineProperty(this, 'options', {
// This fails because of TS 3.7.2 useDefineForClassFields
// Ref: https://github.com/microsoft/TypeScript/issues/34972
enumerable: false,
value: requestOrResponse
value: self.options
});

Object.defineProperty(this, 'response', {
} else {
Object.defineProperty(this, 'options', {
// This fails because of TS 3.7.2 useDefineForClassFields
// Ref: https://github.com/microsoft/TypeScript/issues/34972
enumerable: false,
value: requestOrResponse[kResponse]
value: self
});
}

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

constructor(response: Response, maxRedirects: number, options: NormalizedOptions) {
super(`Redirected ${maxRedirects} times. Aborting.`, {}, options);
constructor(request: Request) {
super(`Redirected ${request.options.maxRedirects} times. Aborting.`, {}, request);
this.name = 'MaxRedirectsError';

Object.defineProperty(this, 'response', {
enumerable: false,
value: response
});
}
}

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

constructor(response: Response, options: NormalizedOptions) {
super(`Response code ${response.statusCode} (${response.statusMessage!})`, {}, options);
constructor(response: Response) {
super(`Response code ${response.statusCode} (${response.statusMessage!})`, {}, response.request);
this.name = 'HTTPError';

Object.defineProperty(this, 'response', {
enumerable: false,
value: response
});
}
}

export class CacheError extends RequestError {
constructor(error: Error, options: NormalizedOptions) {
super(error.message, error, options);
constructor(error: Error, request: Request) {
super(error.message, error, request);
this.name = 'CacheError';
}
}

export class UploadError extends RequestError {
constructor(error: Error, options: NormalizedOptions, request: Request) {
super(error.message, error, options, request);
constructor(error: Error, request: Request) {
super(error.message, error, request);
this.name = 'UploadError';
}
}
Expand All @@ -437,17 +398,17 @@ export class TimeoutError extends RequestError {
readonly timings: Timings;
readonly event: string;

constructor(error: TimedOutTimeoutError, timings: Timings, options: NormalizedOptions) {
super(error.message, error, options);
constructor(error: TimedOutTimeoutError, timings: Timings, request: Request) {
super(error.message, error, request);
this.name = 'TimeoutError';
this.event = error.event;
this.timings = timings;
}
}

export class ReadError extends RequestError {
constructor(error: Error, options: NormalizedOptions, response: Response) {
super(error.message, error, options, response);
constructor(error: Error, request: Request) {
super(error.message, error, request);
this.name = 'ReadError';
}
}
Expand Down Expand Up @@ -982,7 +943,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
});

response.on('error', (error: Error) => {
this._beforeError(new ReadError(error, options, response as Response));
this._beforeError(new ReadError(error, this));
});

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

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

if (this.redirects.length >= options.maxRedirects) {
this._beforeError(new MaxRedirectsError(typedResponse, options.maxRedirects, options));
this._beforeError(new MaxRedirectsError(this));
return;
}

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

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

request.once('error', (error: Error) => {
if (error instanceof TimedOutTimeoutError) {
error = new TimeoutError(error, this.timings!, options);
error = new TimeoutError(error, this.timings!, this);
} else {
error = new RequestError(error.message, error, options, this);
error = new RequestError(error.message, error, this);
}

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

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

async _createCacheableRequest(url: URL, options: RequestOptions): Promise<ClientRequest | ResponseLike> {
return new Promise<ClientRequest | ResponseLike>((resolve, reject) => {
// TODO: Remove `utils/url-to-options.ts` when `cacheable-request` is fixed
Object.assign(options, urlToOptions(url));

// `http-cache-semantics` checks this
delete (options as unknown as NormalizedOptions).url;

// This is ugly
const cacheRequest = cacheableStore.get((options as any).cache)!(options, resolve as any);

// Restore options
(options as unknown as NormalizedOptions).url = url;

cacheRequest.once('error', (error: Error) => {
if (error instanceof CacheableRequest.RequestError) {
// TODO: `options` should be `normalizedOptions`
reject(new RequestError(error.message, error, this));
return;
}

// TODO: `options` should be `normalizedOptions`
reject(new CacheError(error, this));
});
cacheRequest.once('request', resolve);
});
}

async _makeRequest(): Promise<void> {
const {options} = this;
const {url, headers, request, agent, timeout} = options;
Expand Down Expand Up @@ -1266,7 +1255,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
}

const realFn = options.request ?? fallbackFn;
const fn = options.cache ? cacheFn : realFn;
const fn = options.cache ? this._createCacheableRequest.bind(this) : realFn;

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

throw new RequestError(error.message, error, options, this);
throw new RequestError(error.message, error, this);
}
}

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

if (!(error instanceof RequestError)) {
error = new RequestError(error.message, error, this.options, this);
error = new RequestError(error.message, error, this);
}

try {
Expand All @@ -1338,7 +1327,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
error = await hook(error as RequestError);
}
} catch (error_) {
error = new RequestError(error_.message, error_, this.options, this);
error = new RequestError(error_.message, error_, this);
}

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

if (error !== null && !is.undefined(error) && !(error instanceof RequestError)) {
error = new RequestError(error.message, error, this.options, this);
error = new RequestError(error.message, error, this);
}

callback(error);
Expand Down
12 changes: 12 additions & 0 deletions test/error.ts
View file
Expand Up @@ -192,6 +192,18 @@ test('normalization errors using convenience methods', async t => {
await t.throwsAsync(got(url).json().text().buffer(), {message: `Invalid URL: ${url}`});
});

test('errors can have request property', withServer, async (t, server, got) => {
server.get('/', (_request, response) => {
response.statusCode = 404;
response.end();
});

const error = await t.throwsAsync<HTTPError>(got(''));

t.truthy(error.response);
t.truthy(error.request!.downloadProgress);
});

// Fails randomly on Node 10:
// Blocked by https://github.com/istanbuljs/nyc/issues/619
// eslint-disable-next-line ava/no-skip-test
Expand Down

0 comments on commit af0b147

Please sign in to comment.