From 47ae05352b1a86436823bdcb1b6c075000a71917 Mon Sep 17 00:00:00 2001 From: Roman Kuznetsov Date: Wed, 4 Sep 2024 18:36:20 +0300 Subject: [PATCH 1/3] fix: remove extra stack frames --- src/browser/stacktrace/utils.ts | 45 ++++++++++++++++----- src/worker/runner/test-runner/index.js | 4 +- test/src/browser/stacktrace/utils.ts | 56 ++++++++++++++++++++++++-- 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/browser/stacktrace/utils.ts b/src/browser/stacktrace/utils.ts index 20a2be9d8..aebc5db7a 100644 --- a/src/browser/stacktrace/utils.ts +++ b/src/browser/stacktrace/utils.ts @@ -144,7 +144,7 @@ export const applyStackTraceIfBetter = (error: Error, stack: RawStackFrames): Er return error; }; -export const filterExtraWdioFrames = (error: Error): Error => { +export const filterExtraStackFrames = (error: Error): Error => { if (!error || !error.message || !error.stack) { return error; } @@ -158,21 +158,48 @@ export const filterExtraWdioFrames = (error: Error): Error => { return error; } - const isWdioFrame = (frame: StackFrame): boolean => { - return Boolean(frame.fileName && frame.fileName.includes("/node_modules/webdriverio/")); - }; + // If we found something more relevant (e.g. user's code), we can remove useless testplane internal frames + // If we haven't, we keep testplane internal frames + const shouldDropTestplaneInternalFrames = + getStackTraceRelevance(error) > FRAME_RELEVANCE.projectInternals.value; - const isIgnoredFunction = (frame: StackFrame): boolean => { - const funcName = frame.functionName; + const isIgnoredWebdriverioFrame = (frame: StackFrame): boolean => { + const isWebdriverioFrame = frame.fileName && frame.fileName.includes("/node_modules/webdriverio/"); + const fnName = frame.functionName; - if (!funcName) { + if (!isWebdriverioFrame || !fnName) { return false; } - return Boolean(WDIO_IGNORED_STACK_FUNCTIONS.some(fn => fn === funcName || "async " + fn === funcName)); + return WDIO_IGNORED_STACK_FUNCTIONS.some(fn => fn === fnName || "async " + fn === fnName); + }; + + const isWdioUtilsFrame = (frame: StackFrame): boolean => { + return Boolean(frame.fileName && frame.fileName.includes("/node_modules/@wdio/utils/")); }; - const shouldIncludeFrame = (frame: StackFrame): boolean => !isWdioFrame(frame) || !isIgnoredFunction(frame); + const isTestplaneExtraInternalFrame = (frame: StackFrame): boolean => { + const testplaneExtraInternalFramePaths = [ + "/node_modules/testplane/src/browser/history/", + "/node_modules/testplane/src/browser/stacktrace/", + ]; + + return Boolean( + frame.fileName && testplaneExtraInternalFramePaths.some(path => frame.fileName?.includes(path)), + ); + }; + + const shouldIncludeFrame = (frame: StackFrame): boolean => { + if (isIgnoredWebdriverioFrame(frame) || isWdioUtilsFrame(frame)) { + return false; + } + + if (shouldDropTestplaneInternalFrames && isTestplaneExtraInternalFrame(frame)) { + return false; + } + + return true; + }; const framesFiltered = rawFramesArr.filter((_, i) => shouldIncludeFrame(framesParsed[i])).join("\n"); diff --git a/src/worker/runner/test-runner/index.js b/src/worker/runner/test-runner/index.js index d7e4bf141..039f0af6d 100644 --- a/src/worker/runner/test-runner/index.js +++ b/src/worker/runner/test-runner/index.js @@ -9,7 +9,7 @@ const OneTimeScreenshooter = require("./one-time-screenshooter"); const { AssertViewError } = require("../../../browser/commands/assert-view/errors/assert-view-error"); const history = require("../../../browser/history"); const { SAVE_HISTORY_MODE } = require("../../../constants/config"); -const { filterExtraWdioFrames } = require("../../../browser/stacktrace/utils"); +const { filterExtraStackFrames } = require("../../../browser/stacktrace/utils"); const { extendWithCodeSnippet } = require("../../../error-snippets"); const { TestplaneInternalError } = require("../../../errors"); @@ -95,7 +95,7 @@ module.exports = class TestRunner extends Runner { this._browserAgent.freeBrowser(this._browser); if (error) { - filterExtraWdioFrames(error); + filterExtraStackFrames(error); await extendWithCodeSnippet(error); diff --git a/test/src/browser/stacktrace/utils.ts b/test/src/browser/stacktrace/utils.ts index 63d294efd..5685451bc 100644 --- a/test/src/browser/stacktrace/utils.ts +++ b/test/src/browser/stacktrace/utils.ts @@ -2,7 +2,7 @@ import { ShallowStackFrames, applyStackTraceIfBetter, captureRawStackFrames, - filterExtraWdioFrames, + filterExtraStackFrames, } from "../../../../src/browser/stacktrace/utils"; type AnyFunc = (...args: any[]) => unknown; // eslint-disable-line @typescript-eslint/no-explicit-any @@ -56,7 +56,7 @@ describe("stacktrace/utils", () => { }); }); - describe("filterExtraWdioFrames", () => { + describe("filterExtraStackFrames", () => { it("should filter out internal wdio frames", () => { const error = new Error("my\nmulti-line\nerror\nmessage"); const errorStack = [ @@ -72,7 +72,7 @@ describe("stacktrace/utils", () => { ].join("\n"); error.stack = `${error.name}: ${error.message}\n${errorStack}`; - filterExtraWdioFrames(error); + filterExtraStackFrames(error); const expectedStack = [ "Error: my\nmulti-line\nerror\nmessage", @@ -84,8 +84,56 @@ describe("stacktrace/utils", () => { assert.equal(error.stack, expectedStack); }); + it("should filter out internal testplane frames", () => { + const error = new Error("my\nmulti-line\nerror\nmessage"); + const errorStack = [ + "at fn (projectDir/node_modules/testplane/src/browser/history/index.js:80:27)", + "at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)", + "at runWithHistoryHooks (projectDir/node_modules/testplane/src/browser/history/index.js:32:12)", + "at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/history/index.js:77:20)", + "at fn (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:53:39)", + "at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)", + "at runWithStacktraceHooks (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:24:24)", + "at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:51:46)", + "at Object. (projectDir/features/feature/test-name.testplane.js:16:28)", + ].join("\n"); + + error.stack = `${error.name}: ${error.message}\n${errorStack}`; + filterExtraStackFrames(error); + + const expectedStack = [ + "Error: my\nmulti-line\nerror\nmessage", + "at Object. (projectDir/features/feature/test-name.testplane.js:16:28)", + ].join("\n"); + + assert.equal(error.stack, expectedStack); + }); + + it("should not filter out internal testplane frames if there is no user's code frame", () => { + const error = new Error("my\nmulti-line\nerror\nmessage"); + const errorStack = [ + "at fn (projectDir/node_modules/testplane/src/browser/history/index.js:80:27)", + "at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)", + "at runWithHistoryHooks (projectDir/node_modules/testplane/src/browser/history/index.js:32:12)", + "at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/history/index.js:77:20)", + "at fn (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:53:39)", + "at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)", + "at runWithStacktraceHooks (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:24:24)", + "at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:51:46)", + "at Some.internal (projectDir/node_modules/foo/bar/baz/index.ts:51:46)", + "at Internal.metod (projectDir/node_modules/qux/index.ts:51:46)", + ].join("\n"); + + error.stack = `${error.name}: ${error.message}\n${errorStack}`; + filterExtraStackFrames(error); + + const expectedStack = "Error: my\nmulti-line\nerror\nmessage" + "\n" + errorStack; + + assert.equal(error.stack, expectedStack); + }); + it("should not throw on bad input", () => { - assert.doesNotThrow(() => filterExtraWdioFrames("error-message" as unknown as Error)); + assert.doesNotThrow(() => filterExtraStackFrames("error-message" as unknown as Error)); }); }); From f236b34838c581d8829a00526bfca647314fe7c4 Mon Sep 17 00:00:00 2001 From: Roman Kuznetsov Date: Thu, 5 Sep 2024 01:39:45 +0300 Subject: [PATCH 2/3] fix: ensure raw stack frames are matching parsed ones --- src/browser/stacktrace/constants.ts | 4 ++++ src/browser/stacktrace/utils.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/browser/stacktrace/constants.ts b/src/browser/stacktrace/constants.ts index 6b846e04e..d5624b2fd 100644 --- a/src/browser/stacktrace/constants.ts +++ b/src/browser/stacktrace/constants.ts @@ -6,3 +6,7 @@ export const WDIO_IGNORED_STACK_FUNCTIONS = [ "Element.newCommand", "Element.elementErrorHandlerCallbackFn", ]; +// Other frames are being filtered out by error-stack-parser +// Declared at https://github.com/stacktracejs/error-stack-parser/blob/v2.1.4/error-stack-parser.js#L17 +// Used at https://github.com/stacktracejs/error-stack-parser/blob/v2.1.4/error-stack-parser.js#L52-L54 +export const STACK_FRAME_REG_EXP = /^\s*at .*(\S+:\d+|\(native\))/m; diff --git a/src/browser/stacktrace/utils.ts b/src/browser/stacktrace/utils.ts index aebc5db7a..be2d3afb7 100644 --- a/src/browser/stacktrace/utils.ts +++ b/src/browser/stacktrace/utils.ts @@ -3,7 +3,7 @@ import ErrorStackParser from "error-stack-parser"; import type { SetRequired } from "type-fest"; import logger from "../../utils/logger"; import { softFileURLToPath } from "../../utils/fs"; -import { WDIO_IGNORED_STACK_FUNCTIONS, WDIO_STACK_TRACE_LIMIT } from "./constants"; +import { STACK_FRAME_REG_EXP, WDIO_IGNORED_STACK_FUNCTIONS, WDIO_STACK_TRACE_LIMIT } from "./constants"; export type RawStackFrames = string; @@ -151,7 +151,7 @@ export const filterExtraStackFrames = (error: Error): Error => { try { const rawFrames = getErrorRawStackFrames(error as ErrorWithStack); - const rawFramesArr = rawFrames.split("\n"); + const rawFramesArr = rawFrames.split("\n").filter(frame => STACK_FRAME_REG_EXP.test(frame)); const framesParsed = ErrorStackParser.parse(error); if (rawFramesArr.length !== framesParsed.length) { From 448648141766b08d9405ec9ccd4ec4baa5db9e10 Mon Sep 17 00:00:00 2001 From: Roman Kuznetsov Date: Thu, 5 Sep 2024 02:44:53 +0300 Subject: [PATCH 3/3] fix: detect overwritten error message --- src/browser/stacktrace/utils.ts | 8 +++++++- test/src/browser/stacktrace/utils.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/browser/stacktrace/utils.ts b/src/browser/stacktrace/utils.ts index be2d3afb7..a906ecb9f 100644 --- a/src/browser/stacktrace/utils.ts +++ b/src/browser/stacktrace/utils.ts @@ -47,7 +47,13 @@ const getErrorRawStackFrames = (e: ErrorWithStack): RawStackFrames => { const errorMessageStackIndex = e.stack.indexOf(e.message); const errorMessageEndsStackIndex = e.stack.indexOf("\n", errorMessageStackIndex + e.message.length); - return e.stack.slice(errorMessageEndsStackIndex + 1); + if (errorMessageStackIndex !== -1) { + return e.stack.slice(errorMessageEndsStackIndex + 1); + } + + const stackTraceRegExpResult = STACK_FRAME_REG_EXP.exec(e.stack); + + return stackTraceRegExpResult ? e.stack.slice(stackTraceRegExpResult.index) : ""; }; export const captureRawStackFrames = (filterFunc?: (...args: unknown[]) => unknown): RawStackFrames => { diff --git a/test/src/browser/stacktrace/utils.ts b/test/src/browser/stacktrace/utils.ts index 5685451bc..630b0148d 100644 --- a/test/src/browser/stacktrace/utils.ts +++ b/test/src/browser/stacktrace/utils.ts @@ -109,6 +109,32 @@ describe("stacktrace/utils", () => { assert.equal(error.stack, expectedStack); }); + it("should work with overwritten multiline error message", () => { + const error = new Error("foo"); + const errorStack = [ + "at fn (projectDir/node_modules/testplane/src/browser/history/index.js:80:27)", + "at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)", + "at runWithHistoryHooks (projectDir/node_modules/testplane/src/browser/history/index.js:32:12)", + "at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/history/index.js:77:20)", + "at fn (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:53:39)", + "at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)", + "at runWithStacktraceHooks (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:24:24)", + "at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:51:46)", + "at Object. (projectDir/features/feature/test-name.testplane.js:16:28)", + ].join("\n"); + + error.stack = `${error.name}: ${error.message}\n${errorStack}`; + error.message = "some/\nnew\nerror\nmessage"; + filterExtraStackFrames(error); + + const expectedStack = [ + "Error: some/\nnew\nerror\nmessage", + "at Object. (projectDir/features/feature/test-name.testplane.js:16:28)", + ].join("\n"); + + assert.equal(error.stack, expectedStack); + }); + it("should not filter out internal testplane frames if there is no user's code frame", () => { const error = new Error("my\nmulti-line\nerror\nmessage"); const errorStack = [