From 3f7c6a8bd7d3a37c8d06bc73e65f9a41300f3f36 Mon Sep 17 00:00:00 2001 From: Rahul Suryakanth Date: Sun, 3 May 2020 20:33:04 -0400 Subject: [PATCH 1/7] Improve error stack traces for async errors --- src/wait-for.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/wait-for.js b/src/wait-for.js index a7a35f68..0b611c4a 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -63,7 +63,16 @@ function waitFor( } function onTimeout() { - onDone(lastError || timedOutError, null) + let error = timedOutError + if (lastError) { + error = lastError + const userStackTrace = timedOutError.stack + .split('\n') + .slice(1) + .join('\n') + error.stack = `${error.stack.split('\n')[0]}\n${userStackTrace}` + } + onDone(error, null) } }) } From ad37a88da378306f0e3a560b507e8a910b6ca25a Mon Sep 17 00:00:00 2001 From: Rahul Suryakanth Date: Mon, 4 May 2020 13:02:11 -0400 Subject: [PATCH 2/7] Updated the config flag based on the feedback --- src/__tests__/wait-for.js | 11 +++++++++++ src/config.js | 2 ++ src/wait-for.js | 13 ++++++++----- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/__tests__/wait-for.js b/src/__tests__/wait-for.js index 9faf9049..0acdb947 100644 --- a/src/__tests__/wait-for.js +++ b/src/__tests__/wait-for.js @@ -34,6 +34,17 @@ test('uses generic error if there was no last error', async () => { expect(result).toMatchInlineSnapshot(`[Error: Timed out in waitFor.]`) }) +test('uses full stack error trace when showOriginalStackTrace present', async () => { + const error = new Error('Throws the full stack trace') + const result = await waitFor( + () => { + throw error + }, + {timeout: 8, interval: 5, showOriginalStackTrace: true}, + ).catch(e => e) + expect(result).toBe(error) +}) + test('throws nice error if provided callback is not a function', () => { const {queryByTestId} = renderIntoDocument(`
diff --git a/src/config.js b/src/config.js index 00153963..248c9008 100644 --- a/src/config.js +++ b/src/config.js @@ -16,6 +16,8 @@ let config = { asyncWrapper: cb => cb(), // default value for the `hidden` option in `ByRole` queries defaultHidden: false, + //showOriginalStackTrace flag to show the full error stack traces for async errors + showOriginalStackTrace: false, // called when getBy* queries fail. (message, container) => Error getElementError(message, container) { diff --git a/src/wait-for.js b/src/wait-for.js index 0b611c4a..62d278da 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -13,6 +13,7 @@ function waitFor( { container = getDocument(), timeout = getConfig().asyncUtilTimeout, + showOriginalStackTrace = getConfig().showOriginalStackTrace, interval = 50, mutationObserverOptions = { subtree: true, @@ -66,11 +67,13 @@ function waitFor( let error = timedOutError if (lastError) { error = lastError - const userStackTrace = timedOutError.stack - .split('\n') - .slice(1) - .join('\n') - error.stack = `${error.stack.split('\n')[0]}\n${userStackTrace}` + if (showOriginalStackTrace) { + const userStackTrace = timedOutError.stack + .split('\n') + .slice(1) + .join('\n') + error.stack = `${error.stack.split('\n')[0]}\n${userStackTrace}` + } } onDone(error, null) } From da4e1ffeb4c1ee03989afe85c1111bf420130ba6 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Mon, 4 May 2020 12:28:39 -0600 Subject: [PATCH 3/7] Update src/__tests__/wait-for.js --- src/__tests__/wait-for.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/wait-for.js b/src/__tests__/wait-for.js index 0acdb947..a49642cc 100644 --- a/src/__tests__/wait-for.js +++ b/src/__tests__/wait-for.js @@ -42,7 +42,7 @@ test('uses full stack error trace when showOriginalStackTrace present', async () }, {timeout: 8, interval: 5, showOriginalStackTrace: true}, ).catch(e => e) - expect(result).toBe(error) + expect(result.stack).toBe(error.stack) }) test('throws nice error if provided callback is not a function', () => { From 3bec76b17f72e13f8be23de9d325198ee249e9a0 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 5 May 2020 11:02:55 -0600 Subject: [PATCH 4/7] improve stac trace stuff --- src/__tests__/wait-for.js | 30 +++++++++++++++++++++++++++++- src/wait-for.js | 36 +++++++++++++++++++++++++----------- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/__tests__/wait-for.js b/src/__tests__/wait-for.js index a49642cc..f7c1ec7a 100644 --- a/src/__tests__/wait-for.js +++ b/src/__tests__/wait-for.js @@ -36,13 +36,41 @@ test('uses generic error if there was no last error', async () => { test('uses full stack error trace when showOriginalStackTrace present', async () => { const error = new Error('Throws the full stack trace') + // even if the error is a TestingLibraryElementError + error.name = 'TestingLibraryElementError' + const originalStackTrace = error.stack const result = await waitFor( () => { throw error }, {timeout: 8, interval: 5, showOriginalStackTrace: true}, ).catch(e => e) - expect(result.stack).toBe(error.stack) + expect(result.stack).toBe(originalStackTrace) +}) + +test('does not change the stack trace if the thrown error is not a TestingLibraryElementError', async () => { + const error = new Error('Throws the full stack trace') + const originalStackTrace = error.stack + const result = await waitFor( + () => { + throw error + }, + {timeout: 8, interval: 5}, + ).catch(e => e) + expect(result.stack).toBe(originalStackTrace) +}) + +test('provides a stack trace from the if the throw error is a TestingLibraryElementError', async () => { + const error = new Error('Throws the full stack trace') + error.name = 'TestingLibraryElementError' + const originalStackTrace = error.stack + const result = await waitFor( + () => { + throw error + }, + {timeout: 8, interval: 5}, + ).catch(e => e) + expect(result.stack).not.toBe(originalStackTrace) }) test('throws nice error if provided callback is not a function', () => { diff --git a/src/wait-for.js b/src/wait-for.js index 62d278da..f01aa886 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -8,6 +8,12 @@ import { } from './helpers' import {getConfig} from './config' +// This is so the stack trace the developer sees is one that's +// closer to their code (because async stack traces are hard to follow). +function copyStackTrace(target, source) { + target.stack = source.stack.replace(source.message, target.message) +} + function waitFor( callback, { @@ -21,14 +27,13 @@ function waitFor( attributes: true, characterData: true, }, + stackTraceError, } = {}, ) { if (typeof callback !== 'function') { throw new TypeError('Received `callback` arg must be a function') } - // created here so we get a nice stacktrace - const timedOutError = new Error('Timed out in waitFor.') if (interval < 1) interval = 1 return new Promise((resolve, reject) => { let lastError @@ -64,15 +69,19 @@ function waitFor( } function onTimeout() { - let error = timedOutError + let error if (lastError) { error = lastError - if (showOriginalStackTrace) { - const userStackTrace = timedOutError.stack - .split('\n') - .slice(1) - .join('\n') - error.stack = `${error.stack.split('\n')[0]}\n${userStackTrace}` + if ( + !showOriginalStackTrace && + error.name === 'TestingLibraryElementError' + ) { + copyStackTrace(error, stackTraceError) + } + } else { + error = new Error('Timed out in waitFor.') + if (!showOriginalStackTrace) { + copyStackTrace(error, stackTraceError) } } onDone(error, null) @@ -80,8 +89,13 @@ function waitFor( }) } -function waitForWrapper(...args) { - return getConfig().asyncWrapper(() => waitFor(...args)) +function waitForWrapper(callback, options) { + // create the error here so its stack trace is as close to the + // calling code as possible + const stackTraceError = new Error('STACK_TRACE_MESSAGE') + return getConfig().asyncWrapper(() => + waitFor(callback, {stackTraceError, ...options}), + ) } let hasWarned = false From cd903dc5b9ba3d333f86bb6257760dfa284805a8 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 5 May 2020 11:04:24 -0600 Subject: [PATCH 5/7] fix test title --- src/__tests__/wait-for.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/__tests__/wait-for.js b/src/__tests__/wait-for.js index f7c1ec7a..70c4ac79 100644 --- a/src/__tests__/wait-for.js +++ b/src/__tests__/wait-for.js @@ -60,7 +60,7 @@ test('does not change the stack trace if the thrown error is not a TestingLibrar expect(result.stack).toBe(originalStackTrace) }) -test('provides a stack trace from the if the throw error is a TestingLibraryElementError', async () => { +test('provides an improved stack trace if the thrown error is a TestingLibraryElementError', async () => { const error = new Error('Throws the full stack trace') error.name = 'TestingLibraryElementError' const originalStackTrace = error.stack @@ -70,6 +70,8 @@ test('provides a stack trace from the if the throw error is a TestingLibraryElem }, {timeout: 8, interval: 5}, ).catch(e => e) + // too hard to test that the stack trace is what we want it to be + // so we'll just make sure that it's not the same as the origianl expect(result.stack).not.toBe(originalStackTrace) }) From 0fa71533827580d4e6ae7972f38ef9b2c4f96a49 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 5 May 2020 11:07:01 -0600 Subject: [PATCH 6/7] move option --- src/wait-for.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wait-for.js b/src/wait-for.js index f01aa886..f00c6256 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -20,6 +20,7 @@ function waitFor( container = getDocument(), timeout = getConfig().asyncUtilTimeout, showOriginalStackTrace = getConfig().showOriginalStackTrace, + stackTraceError, interval = 50, mutationObserverOptions = { subtree: true, @@ -27,7 +28,6 @@ function waitFor( attributes: true, characterData: true, }, - stackTraceError, } = {}, ) { if (typeof callback !== 'function') { From 3b780fae4859fdaff78ac038badc75ec3e4193ed Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 5 May 2020 11:19:23 -0600 Subject: [PATCH 7/7] fix code coverage --- src/__tests__/wait-for.js | 13 ++++++++++++- src/wait-for.js | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/__tests__/wait-for.js b/src/__tests__/wait-for.js index 70c4ac79..2c7fa964 100644 --- a/src/__tests__/wait-for.js +++ b/src/__tests__/wait-for.js @@ -23,7 +23,7 @@ test('can timeout after the given timeout time', async () => { expect(result).toBe(error) }) -test('uses generic error if there was no last error', async () => { +test('if no error is thrown then throws a timeout error', async () => { const result = await waitFor( () => { // eslint-disable-next-line no-throw-literal @@ -34,6 +34,17 @@ test('uses generic error if there was no last error', async () => { expect(result).toMatchInlineSnapshot(`[Error: Timed out in waitFor.]`) }) +test('if showOriginalStackTrace on a timeout error then the stack trace does not include this file', async () => { + const result = await waitFor( + () => { + // eslint-disable-next-line no-throw-literal + throw undefined + }, + {timeout: 8, interval: 5, showOriginalStackTrace: true}, + ).catch(e => e) + expect(result.stack).not.toMatch(__dirname) +}) + test('uses full stack error trace when showOriginalStackTrace present', async () => { const error = new Error('Throws the full stack trace') // even if the error is a TestingLibraryElementError diff --git a/src/wait-for.js b/src/wait-for.js index f00c6256..931a01a7 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -28,7 +28,7 @@ function waitFor( attributes: true, characterData: true, }, - } = {}, + }, ) { if (typeof callback !== 'function') { throw new TypeError('Received `callback` arg must be a function')