From 6eaa81ba8db87340bbdbd79ef91594803f37b854 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Wed, 24 Apr 2019 13:55:06 +0200 Subject: [PATCH] Move top-level error properties into an `.options` and `.response` property (#773) --- readme.md | 12 +++--- source/as-promise.ts | 4 +- source/errors.ts | 61 +++++++++++------------------- source/normalize-arguments.ts | 15 ++++---- source/request-as-event-emitter.ts | 7 ++-- test/cache.ts | 2 +- test/error.ts | 34 +++++++++-------- test/gzip.ts | 4 +- test/http.ts | 6 +-- test/redirects.ts | 6 +-- test/response-parse.ts | 14 +++---- test/retry.ts | 2 +- 12 files changed, 74 insertions(+), 93 deletions(-) diff --git a/readme.md b/readme.md index a5bc2fcea..ceeac6db5 100644 --- a/readme.md +++ b/readme.md @@ -531,7 +531,7 @@ Type: `Object` **Note:** This is not a [http.ClientRequest](https://nodejs.org/api/http.html#http_class_http_clientrequest). -- `gotOptions` - The options that were set on this request. +- `options` - The Got options that were set on this request. ##### body @@ -747,9 +747,7 @@ The default Got options. ## Errors -Each error contains `host`, `hostname`, `method`, `path`, `protocol`, `url` and `gotOptions` properties to make debugging easier. - -In Promise mode, the `response` is attached to the error. +Each error contains an `options` property which are the options Got used to create a request - just to make debugging easier. #### got.CacheError @@ -765,15 +763,15 @@ When reading from response stream fails. #### got.ParseError -When server response code is 2xx, and parsing body fails. Includes `body`, `statusCode` and `statusMessage` properties. +When server response code is 2xx, and parsing body fails. Includes a `response` property. #### got.HTTPError -When the server response code is not 2xx. Includes `body`, `statusCode`, `statusMessage`, and `redirectUrls` properties. +When the server response code is not 2xx. Includes a `response` property. #### got.MaxRedirectsError -When the server redirects you more than ten times. Includes a `statusCode`, `statusMessage`, and `redirectUrls` property which is an array of the URLs Got was redirected to before giving up. +When the server redirects you more than ten times. Includes a `response` property. #### got.UnsupportedProtocolError diff --git a/source/as-promise.ts b/source/as-promise.ts index a86e25d2f..062525eeb 100644 --- a/source/as-promise.ts +++ b/source/as-promise.ts @@ -80,8 +80,7 @@ export default function asPromise(options: Options) { parseBody(response); } catch (error) { if (statusCode >= 200 && statusCode < 300) { - const parseError = new ParseError(error, statusCode, options, data); - Object.defineProperty(parseError, 'response', {value: response}); + const parseError = new ParseError(error, response, options); reject(parseError); return; } @@ -90,7 +89,6 @@ export default function asPromise(options: Options) { if (statusCode !== 304 && (statusCode < 200 || statusCode > limitStatusCode)) { const error = new HTTPError(response, options); - Object.defineProperty(error, 'response', {value: response}); if (emitter.retry(error) === false) { if (options.throwHttpErrors) { reject(error); diff --git a/source/errors.ts b/source/errors.ts index 871caa1f1..853ae5e6b 100644 --- a/source/errors.ts +++ b/source/errors.ts @@ -1,5 +1,5 @@ import urlLib from 'url'; -import http, {IncomingHttpHeaders} from 'http'; +import http from 'http'; import is from '@sindresorhus/is'; import {Timings} from '@szmarczak/http-timer'; import {Response, Options} from './utils/types'; @@ -10,6 +10,8 @@ type ErrorWithCode = (Error & {code?: string}) | {code?: string}; export class GotError extends Error { code?: string; + options: Options; + constructor(message: string, error: ErrorWithCode, options: Options) { super(message); Error.captureStackTrace(this, this.constructor); @@ -19,15 +21,8 @@ export class GotError extends Error { this.code = error.code; } - Object.assign(this, { - host: options.host, - hostname: options.hostname, - method: options.method, - path: options.path, - socketPath: options.socketPath, - protocol: options.protocol, - url: options.href, - gotOptions: options + Object.defineProperty(this, 'options', { + value: options }); } } @@ -54,29 +49,20 @@ export class ReadError extends GotError { } export class ParseError extends GotError { - body: string | Buffer; - - statusCode: number; + response: Response; - statusMessage?: string; - - constructor(error: Error, statusCode: number, options: Options, data: string | Buffer) { + constructor(error: Error, response: Response, options: Options) { super(`${error.message} in "${urlLib.format(options)}"`, error, options); this.name = 'ParseError'; - this.body = data; - this.statusCode = statusCode; - this.statusMessage = http.STATUS_CODES[this.statusCode]; + + Object.defineProperty(this, 'response', { + value: response + }); } } export class HTTPError extends GotError { - headers?: IncomingHttpHeaders; - - body: string | Buffer; - - statusCode: number; - - statusMessage?: string; + response: Response; constructor(response: Response, options: Options) { const {statusCode} = response; @@ -90,26 +76,23 @@ export class HTTPError extends GotError { super(`Response code ${statusCode} (${statusMessage})`, {}, options); this.name = 'HTTPError'; - this.statusCode = statusCode; - this.statusMessage = statusMessage; - this.headers = response.headers; - this.body = response.body; + + Object.defineProperty(this, 'response', { + value: response + }); } } export class MaxRedirectsError extends GotError { - redirectUrls?: string[]; - - statusMessage?: string; - - statusCode: number; + response: Response; - constructor(statusCode: number, redirectUrls: string[], options: Options) { + constructor(response: Response, options: Options) { super('Redirected 10 times. Aborting.', {}, options); this.name = 'MaxRedirectsError'; - this.statusCode = statusCode; - this.statusMessage = http.STATUS_CODES[this.statusCode]; - this.redirectUrls = redirectUrls; + + Object.defineProperty(this, 'response', { + value: response + }); } } diff --git a/source/normalize-arguments.ts b/source/normalize-arguments.ts index 228922586..b2363706e 100644 --- a/source/normalize-arguments.ts +++ b/source/normalize-arguments.ts @@ -220,17 +220,18 @@ export const normalizeArguments = (url, options, defaults?: any) => { return 0; } - const hasCode = !is.undefined(error.code) && options.retry.errorCodes.has(error.code); - const hasMethod = !is.undefined(error.method) && options.retry.methods.has(error.method); - const hasStatusCode = !is.undefined(error.statusCode) && options.retry.statusCodes.has(error.statusCode); + const hasCode = Reflect.has(error, 'code') && options.retry.errorCodes.has(error.code); + const hasMethod = Reflect.has(error, 'options') && options.retry.methods.has(error.options.method); + const hasStatusCode = Reflect.has(error, 'response') && options.retry.statusCodes.has(error.response.statusCode); if ((!error || !hasCode) && (!hasMethod || !hasStatusCode)) { return 0; } - if (Reflect.has(error, 'headers') && Reflect.has(error.headers, 'retry-after') && retryAfterStatusCodes.has(error.statusCode)) { - let after = Number(error.headers['retry-after']); + const {response} = error; + if (response && Reflect.has(response.headers, 'retry-after') && retryAfterStatusCodes.has(response.statusCode)) { + let after = Number(response.headers['retry-after']); if (is.nan(after)) { - after = Date.parse(error.headers['retry-after']) - Date.now(); + after = Date.parse(response.headers['retry-after']) - Date.now(); } else { after *= 1000; } @@ -242,7 +243,7 @@ export const normalizeArguments = (url, options, defaults?: any) => { return after; } - if (error.statusCode === 413) { + if (response && response.statusCode === 413) { return 0; } diff --git a/source/request-as-event-emitter.ts b/source/request-as-event-emitter.ts index da8e759b6..890388e0a 100644 --- a/source/request-as-event-emitter.ts +++ b/source/request-as-event-emitter.ts @@ -111,14 +111,13 @@ export default (options, input?: TransformStream) => { } const {statusCode} = response; + response.statusMessage = response.statusMessage || http.STATUS_CODES[statusCode]; response.url = currentUrl; response.requestUrl = requestUrl; response.retryCount = retryCount; response.timings = timings; response.redirectUrls = redirects; - response.request = { - gotOptions: options - }; + response.request = {options}; response.isFromCache = response.fromCache || false; delete response.fromCache; @@ -138,7 +137,7 @@ export default (options, input?: TransformStream) => { } if (redirects.length >= 10) { - throw new MaxRedirectsError(statusCode, redirects, options); + throw new MaxRedirectsError(response, options); } // Handles invalid URLs. See https://github.com/sindresorhus/got/issues/604 diff --git a/test/cache.ts b/test/cache.ts index 87a8f04bf..22d906ff3 100644 --- a/test/cache.ts +++ b/test/cache.ts @@ -99,7 +99,7 @@ test('cached response has got options', withServer, async (t, server, got) => { await got(options); const secondResponse = await got(options); - t.is(secondResponse.request.gotOptions.auth, options.auth); + t.is(secondResponse.request.options.auth, options.auth); }); test('cache error throws `got.CacheError`', withServer, async (t, server, got) => { diff --git a/test/error.ts b/test/error.ts index cec766f25..370c28210 100644 --- a/test/error.ts +++ b/test/error.ts @@ -17,14 +17,16 @@ test('properties', withServer, async (t, server, got) => { t.truthy(error); // @ts-ignore t.truthy(error.response); + t.truthy(error.options); + t.false({}.propertyIsEnumerable.call(error, 'options')); t.false({}.propertyIsEnumerable.call(error, 'response')); t.false({}.hasOwnProperty.call(error, 'code')); t.is(error.message, 'Response code 404 (Not Found)'); - t.is(error.host, `${url.hostname}:${url.port}`); - t.is(error.method, 'GET'); - t.is(error.protocol, 'http:'); - t.is(error.url, error.response.requestUrl); - t.is(error.headers.connection, 'close'); + t.is(error.options.host, `${url.hostname}:${url.port}`); + t.is(error.options.method, 'GET'); + t.is(error.options.protocol, 'http:'); + t.is(error.options.url, error.options.requestUrl); + t.is(error.response.headers.connection, 'close'); t.is(error.response.body, 'not'); }); @@ -32,8 +34,8 @@ test('catches dns errors', async t => { const error = await t.throwsAsync(got('http://doesntexist', {retry: 0})) as any; t.truthy(error); t.regex(error.message, /getaddrinfo ENOTFOUND/); - t.is(error.host, 'doesntexist'); - t.is(error.method, 'GET'); + t.is(error.options.host, 'doesntexist'); + t.is(error.options.method, 'GET'); }); test('`options.body` form error message', async t => { @@ -64,9 +66,9 @@ test('default status message', withServer, async (t, server, got) => { const error = await t.throwsAsync(got('')); // @ts-ignore - t.is(error.statusCode, 400); + t.is(error.response.statusCode, 400); // @ts-ignore - t.is(error.statusMessage, 'Bad Request'); + t.is(error.response.statusMessage, 'Bad Request'); }); test('custom status message', withServer, async (t, server, got) => { @@ -78,9 +80,9 @@ test('custom status message', withServer, async (t, server, got) => { const error = await t.throwsAsync(got('')); // @ts-ignore - t.is(error.statusCode, 400); + t.is(error.response.statusCode, 400); // @ts-ignore - t.is(error.statusMessage, 'Something Exploded'); + t.is(error.response.statusMessage, 'Something Exploded'); }); test('custom body', withServer, async (t, server, got) => { @@ -91,9 +93,9 @@ test('custom body', withServer, async (t, server, got) => { const error = await t.throwsAsync(got('')); // @ts-ignore - t.is(error.statusCode, 404); + t.is(error.response.statusCode, 404); // @ts-ignore - t.is(error.body, 'not'); + t.is(error.response.body, 'not'); }); test('contains Got options', withServer, async (t, server, got) => { @@ -108,7 +110,7 @@ test('contains Got options', withServer, async (t, server, got) => { const error = await t.throwsAsync(got(options)); // @ts-ignore - t.is(error.gotOptions.auth, options.auth); + t.is(error.options.auth, options.auth); }); test('empty status message is overriden by the default one', withServer, async (t, server, got) => { @@ -119,9 +121,9 @@ test('empty status message is overriden by the default one', withServer, async ( const error = await t.throwsAsync(got('')); // @ts-ignore - t.is(error.statusCode, 400); + t.is(error.response.statusCode, 400); // @ts-ignore - t.is(error.statusMessage, http.STATUS_CODES[400]); + t.is(error.response.statusMessage, http.STATUS_CODES[400]); }); test('`http.request` error', async t => { diff --git a/test/gzip.ts b/test/gzip.ts index 0a30712af..91ea1ec2d 100644 --- a/test/gzip.ts +++ b/test/gzip.ts @@ -39,7 +39,7 @@ test('handles gzip error', withServer, async (t, server, got) => { const error = await t.throwsAsync(got(''), 'incorrect header check'); // @ts-ignore - t.is(error.path, '/'); + t.is(error.options.path, '/'); t.is(error.name, 'ReadError'); }); @@ -52,7 +52,7 @@ test('handles gzip error - stream', withServer, async (t, server, got) => { const error = await t.throwsAsync(getStream(got.stream('')), 'incorrect header check'); // @ts-ignore - t.is(error.path, '/'); + t.is(error.options.path, '/'); t.is(error.name, 'ReadError'); }); diff --git a/test/http.ts b/test/http.ts index e2351d1ab..6bb672cf4 100644 --- a/test/http.ts +++ b/test/http.ts @@ -32,14 +32,14 @@ test('response has `requestUrl` property', withServer, async (t, server, got) => t.is((await got('empty')).requestUrl, `${server.url}/empty`); }); -test('errors have `statusCode` property', withServer, async (t, server, got) => { +test('http errors have `response` property', withServer, async (t, server, got) => { server.get('/', (_request, response) => { response.statusCode = 404; response.end('not'); }); const error = await t.throwsAsync(got(''), got.HTTPError); - t.is(error.statusCode, 404); + t.is(error.response.statusCode, 404); t.is(error.response.body, 'not'); }); @@ -117,5 +117,5 @@ test('response contains got options', withServer, async (t, server, got) => { auth: 'foo:bar' }; - t.is((await got(options)).request.gotOptions.auth, options.auth); + t.is((await got(options)).request.options.auth, options.auth); }); diff --git a/test/redirects.ts b/test/redirects.ts index 250473124..80e24ffa5 100644 --- a/test/redirects.ts +++ b/test/redirects.ts @@ -79,7 +79,7 @@ test('throws on endless redirects', withServer, async (t, server, got) => { const error = await t.throwsAsync(got(''), 'Redirected 10 times. Aborting.'); // @ts-ignore - t.deepEqual(error.redirectUrls, new Array(10).fill(`${server.url}/`)); + t.deepEqual(error.response.redirectUrls, new Array(10).fill(`${server.url}/`)); }); test('searchParams are not breaking redirects', withServer, async (t, server, got) => { @@ -116,9 +116,9 @@ test('redirects only GET and HEAD requests', withServer, async (t, server, got) }); // @ts-ignore - t.is(error.path, '/'); + t.is(error.options.path, '/'); // @ts-ignore - t.is(error.statusCode, 302); + t.is(error.response.statusCode, 302); }); test('redirects on 303 response even on post, put, delete', withServer, async (t, server, got) => { diff --git a/test/response-parse.ts b/test/response-parse.ts index 2f77b8110..5b53066ed 100644 --- a/test/response-parse.ts +++ b/test/response-parse.ts @@ -55,9 +55,9 @@ test('throws an error on invalid response type', withServer, async (t, server, g const error = await t.throwsAsync(got({responseType: 'invalid'}), /^Failed to parse body of type 'invalid'/); // @ts-ignore - t.true(error.message.includes(error.hostname)); + t.true(error.message.includes(error.options.hostname)); // @ts-ignore - t.is(error.path, '/'); + t.is(error.options.path, '/'); }); test('doesn\'t parse responses without a body', withServer, async (t, server, got) => { @@ -76,9 +76,9 @@ test('wraps parsing errors', withServer, async (t, server, got) => { const error = await t.throwsAsync(got({responseType: 'json'}), got.ParseError); // @ts-ignore - t.true(error.message.includes(error.hostname)); + t.true(error.message.includes(error.options.hostname)); // @ts-ignore - t.is(error.path, '/'); + t.is(error.options.path, '/'); }); test('parses non-200 responses', withServer, async (t, server, got) => { @@ -106,10 +106,10 @@ test('ignores errors on invalid non-200 responses', withServer, async (t, server // @ts-ignore t.is(error.response.body, 'Internal error'); // @ts-ignore - t.is(error.path, '/'); + t.is(error.options.path, '/'); }); -test('errors have `statusCode` property', withServer, async (t, server, got) => { +test('parse errors have `response` property', withServer, async (t, server, got) => { server.get('/', (_request, response) => { response.end('/'); }); @@ -117,7 +117,7 @@ test('errors have `statusCode` property', withServer, async (t, server, got) => const error = await t.throwsAsync(got({responseType: 'json'}), got.ParseError); // @ts-ignore - t.is(error.statusCode, 200); + t.is(error.response.statusCode, 200); }); test('sets correct headers', withServer, async (t, server, got) => { diff --git a/test/retry.ts b/test/retry.ts index cf483d4f5..c64ff62aa 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -103,7 +103,7 @@ test('custom retries', withServer, async (t, server, got) => { } })); // @ts-ignore - t.is(error.statusCode, 500); + t.is(error.response.statusCode, 500); t.true(tried); });