From c15bbe81a76c3d0b759ebf479ac11dec3523eae4 Mon Sep 17 00:00:00 2001 From: tabarra Date: Tue, 6 Oct 2020 23:02:13 -0300 Subject: [PATCH 1/4] Changed error.stack type checking (resolves #1490) --- source/core/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core/index.ts b/source/core/index.ts index 8b8b0966e..eb845facf 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -1222,7 +1222,7 @@ export class RequestError extends Error { this.timings = this.request?.timings; // Recover the original stacktrace - if (!is.undefined(error.stack)) { + if (is.string(error.stack)) { const indexOfMessage = this.stack.indexOf(this.message) + this.message.length; const thisStackTrace = this.stack.slice(indexOfMessage).split('\n').reverse(); const errorStackTrace = error.stack.slice(error.stack.indexOf(error.message!) + error.message!.length).split('\n').reverse(); From 600604ff4e2782c20d245eb8777d723030c61651 Mon Sep 17 00:00:00 2001 From: Andre Tabarra Date: Wed, 7 Oct 2020 11:05:51 -0300 Subject: [PATCH 2/4] RequestError now checks both error.stack and this.stack --- source/core/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/core/index.ts b/source/core/index.ts index eb845facf..b77a68e74 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -1222,7 +1222,7 @@ export class RequestError extends Error { this.timings = this.request?.timings; // Recover the original stacktrace - if (is.string(error.stack)) { + if (!is.undefined(error.stack) && is.string(this.stack)) { const indexOfMessage = this.stack.indexOf(this.message) + this.message.length; const thisStackTrace = this.stack.slice(indexOfMessage).split('\n').reverse(); const errorStackTrace = error.stack.slice(error.stack.indexOf(error.message!) + error.message!.length).split('\n').reverse(); From 2d3043f1b6162781b36ae87a8b66a5aab6dbf905 Mon Sep 17 00:00:00 2001 From: Andre Tabarra Date: Wed, 7 Oct 2020 11:12:58 -0300 Subject: [PATCH 3/4] added TypeError test for #1490 --- test/error.ts | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/error.ts b/test/error.ts index faebb37fd..bb172928e 100644 --- a/test/error.ts +++ b/test/error.ts @@ -261,3 +261,37 @@ test.skip('the old stacktrace is recovered', async t => { // the second `at get` points to the real cause. t.not(error.stack!.indexOf('at get'), error.stack!.lastIndexOf('at get')); }); + +test('no TypeError when Error.captureStackTrace returns an string or array', withServer, async t => { + // Checking default nodejs behavior + Error.captureStackTrace = target => { + Object.assign(target, { + stack: 'Error: Timeout awaiting \'request\' for 1ms\n at ClientRequest. (E:\\_TMP\\index.js:100:25)\n at Object.onceWrapper (events.js:422:26)' + }); + }; + + const errorString = await t.throwsAsync(got('https://example.com', {timeout: 1, retry: 0})); + t.true(errorString.stack!.includes('Error: Timeout awaiting')); + + // Checking fxserver's nodejs behavior + Error.captureStackTrace = target => { + Object.assign(target, { + stack: [ + { + file: 'E:\\_TMP\\got\\dist\\source\\core\\index.js', + line: 100, + name: 'ClientRequest.' + }, + { + file: 'events.js', + line: 422, + name: 'Object.onceWrapper' + } + ] + }); + }; + + const errorArray = await t.throwsAsync(got('https://example.com', {timeout: 1, retry: 0})); + t.true(Array.isArray(errorArray.stack)); +}); + From e319c7b8a1d4b415b8762e7bb66aa94e43430ee8 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Mon, 2 Nov 2020 13:11:01 +0100 Subject: [PATCH 4/4] Fixes --- source/core/index.ts | 2 +- test/error.ts | 91 ++++++++++++++++++++++++++++++-------------- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/source/core/index.ts b/source/core/index.ts index b77a68e74..390a184d9 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -1222,7 +1222,7 @@ export class RequestError extends Error { this.timings = this.request?.timings; // Recover the original stacktrace - if (!is.undefined(error.stack) && is.string(this.stack)) { + if (is.string(error.stack) && is.string(this.stack)) { const indexOfMessage = this.stack.indexOf(this.message) + this.message.length; const thisStackTrace = this.stack.slice(indexOfMessage).split('\n').reverse(); const errorStackTrace = error.stack.slice(error.stack.indexOf(error.message!) + error.message!.length).split('\n').reverse(); diff --git a/test/error.ts b/test/error.ts index bb172928e..9a3161494 100644 --- a/test/error.ts +++ b/test/error.ts @@ -3,6 +3,8 @@ import net = require('net'); import http = require('http'); import stream = require('stream'); import test from 'ava'; +import getStream = require('get-stream'); +import is from '@sindresorhus/is'; import got, {RequestError, HTTPError, TimeoutError} from '../source'; import withServer from './helpers/with-server'; @@ -262,36 +264,69 @@ test.skip('the old stacktrace is recovered', async t => { t.not(error.stack!.indexOf('at get'), error.stack!.lastIndexOf('at get')); }); -test('no TypeError when Error.captureStackTrace returns an string or array', withServer, async t => { - // Checking default nodejs behavior - Error.captureStackTrace = target => { - Object.assign(target, { - stack: 'Error: Timeout awaiting \'request\' for 1ms\n at ClientRequest. (E:\\_TMP\\index.js:100:25)\n at Object.onceWrapper (events.js:422:26)' - }); +test.serial('custom stack trace', withServer, async (t, _server, got) => { + const ErrorCaptureStackTrace = Error.captureStackTrace; + + const enable = () => { + Error.captureStackTrace = (target: {stack: any}) => { + target.stack = [ + 'line 1', + 'line 2' + ]; + }; }; - const errorString = await t.throwsAsync(got('https://example.com', {timeout: 1, retry: 0})); - t.true(errorString.stack!.includes('Error: Timeout awaiting')); - - // Checking fxserver's nodejs behavior - Error.captureStackTrace = target => { - Object.assign(target, { - stack: [ - { - file: 'E:\\_TMP\\got\\dist\\source\\core\\index.js', - line: 100, - name: 'ClientRequest.' - }, - { - file: 'events.js', - line: 422, - name: 'Object.onceWrapper' - } - ] - }); + const disable = () => { + Error.captureStackTrace = ErrorCaptureStackTrace; }; - const errorArray = await t.throwsAsync(got('https://example.com', {timeout: 1, retry: 0})); - t.true(Array.isArray(errorArray.stack)); -}); + // Node.js default behavior + { + const stream = got.stream(''); + stream.destroy(new Error('oh no')); + + const caught = await t.throwsAsync(getStream(stream)); + t.is(is(caught.stack), 'string'); + } + + // Passing a custom error + { + enable(); + const error = new Error('oh no'); + disable(); + + const stream = got.stream(''); + stream.destroy(error); + + const caught = await t.throwsAsync(getStream(stream)); + t.is(is(caught.stack), 'string'); + } + + // Custom global behavior + { + enable(); + const error = new Error('oh no'); + const stream = got.stream(''); + stream.destroy(error); + + const caught = await t.throwsAsync(getStream(stream)); + t.is(is(caught.stack), 'Array'); + + disable(); + } + + // Passing a default error that needs some processing + { + const error = new Error('oh no'); + enable(); + + const stream = got.stream(''); + stream.destroy(error); + + const caught = await t.throwsAsync(getStream(stream)); + t.is(is(caught.stack), 'Array'); + + disable(); + } +});