core: expose error stack on errored audits - #14491
Conversation
| const evaluationError = new Error(`Runtime.evaluate exception: ${message}`); | ||
| if (ex.exception?.description && ex.stackTrace) { | ||
| // The description contains the stack trace formatted as expected, if present. | ||
| evaluationError.stack = ex.exception.description; |
There was a problem hiding this comment.
Could we combine the error stacks so the error stack also tells us where in LH code it occured?
Ex: https://github.com/GoogleChrome/lighthouse/blob/main/core/scripts/pptr-run-devtools.js#L87
There was a problem hiding this comment.
If we want that, we should just recurse the error.cause chain and join all the stacks for errorStack.
EDIT: read the code, don't see what that has to do with stacks but ya adding maybe the first few characters of the expression could be good for context here. We don't have the name/Function at this layer, and the entire thing would be way too long (axe...)
There was a problem hiding this comment.
I think that's what @adamraine meant, otherwise you lose the evaluate() call stack on the LH side. In theory not a huge deal: if you're getting an error this early, you were probably just editing the js and know where the call was coming from, but it can be an issue for evaluate calls originating in many places
There was a problem hiding this comment.
I decided that modifying stack is probably a bad idea. Just added the extra details to the error message instead, and then was pleasantly surprised to see that v8 will parse the message for additional stack frames and add it to stacks itself!
Here's what printing evaluationError looks like now:
Error: Runtime.evaluate exception
Expression: (() => { return (function computeBenchmarkIndex() { /** * The GC-heavy benchmark that creates a stri
---- (elided)
ReferenceError: Prommmise is not defined
at wrapInNativePromise (_lighthouse-eval.js:8:9)
at _lighthouse-eval.js:83:8
at ExecutionContext._evaluateInContext (file:///Users/cjamcl/src/lighthouse/core/gather/driver/execution-context.js:139:31)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async ExecutionContext.evaluateAsync (file:///Users/cjamcl/src/lighthouse/core/gather/driver/execution-context.js:173:14)
at async getBenchmarkIndex (file:///Users/cjamcl/src/lighthouse/core/gather/driver/environment.js:54:20)
at async getBaseArtifacts (file:///Users/cjamcl/src/lighthouse/core/gather/base-artifacts.js:21:26)
at async _setup (file:///Users/cjamcl/src/lighthouse/core/gather/navigation-runner.js:56:25)
at async file:///Users/cjamcl/src/lighthouse/core/gather/navigation-runner.js:336:31
at async Function.gather (file:///Users/cjamcl/src/lighthouse/core/runner.js:172:21)
at async navigationGather (file:///Users/cjamcl/src/lighthouse/core/gather/navigation-runner.js:318:21)
at async navigation (file:///Users/cjamcl/src/lighthouse/core/index.js:94:24)
There was a problem hiding this comment.
did something change? Here's the error stack I get on hreflang if I make an error in LinkElements:
errorMessage: Required LinkElements gatherer encountered an error: getElementsInDocumentz is not defined
errorStack: Error: LighthouseError: ERRORED_REQUIRED_ARTIFACT
at Runner._runAudit (~/lighthouse/core/runner.js:431:25)
at Runner._runAudits (~/lighthouse/core/runner.js:370:40)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Runner.audit (~/lighthouse/core/runner.js:62:32)
at async runLighthouse (~/lighthouse/cli/run.js:250:8)
at async ~/lighthouse/cli/index.js:10:1
at <anonymous>:1:1There was a problem hiding this comment.
ah, to get a populated response.exceptionDetails you actually need invalid JS or the like, not just an error. Adding some random characters, I now get
errorMessage:
Required LinkElements gatherer encountered an error: Runtime.evaluate exception
Expression: (() => { function getNodeDetails(element) { function truncate(string, characterLimit) { const Util = ---- (elided)
Parse error at: 294:6
SyntaxError: Unexpected token '*'
at <anonymous>:1:1
errorStack:
LighthouseError: ERRORED_REQUIRED_ARTIFACT
at Runner._runAudit (~/lighthouse/core/runner.js:431:25)
at Runner._runAudits (~/lighthouse/core/runner.js:370:40)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Runner.audit (~/lighthouse/core/runner.js:62:32)
at async runLighthouse (~/lighthouse/cli/run.js:250:8)
at async ~/lighthouse/cli/index.js:10:1
at <anonymous>:1:1errorMessage seems to have some of the useful new bits, but formatted poorly for an error message.
Breaking in _evaluateInContext I see the expanded evaluationError
Error: Runtime.evaluate exception
Expression: (() => { function getNodeDetails(element) { function truncate(string, characterLimit) { const Util =
---- (elided)
Parse error at: 294:6
SyntaxError: Unexpected token '*'
at ExecutionContext._evaluateInContext (~/lighthouse/core/gather/driver/execution-context.js:140:31)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async ExecutionContext.evaluateAsync (~/lighthouse/core/gather/driver/execution-context.js:173:14)
at async LinkElements._getArtifact (~/lighthouse/core/gather/gatherers/link-elements.js:183:17)
at async ~/lighthouse/core/gather/runner-helpers.js:101:24
at async collectPhaseArtifacts (~/lighthouse/core/gather/runner-helpers.js:118:5)
at async _computeNavigationResult (~/lighthouse/core/gather/navigation-runner.js:200:5)
at async _navigations (~/lighthouse/core/gather/navigation-runner.js:297:30)
at async ~/lighthouse/core/gather/navigation-runner.js:370:27
at async Runner.gather (~/lighthouse/core/runner.js:218:21) {stack: 'Error: Runtime.evaluate exception
Expression:…~/lighthouse/core/runner.js:218:21)', message: 'Runtime.evaluate exception
Expression: (() =… at: 294:6
SyntaxError: Unexpected token '*''}
Looking at just evaluationError.stack:
'Error: Runtime.evaluate exception
Expression: (() => { function getNodeDetails(element) { function truncate(string, characterLimit) { const Util =
---- (elided)
Parse error at: 294:6
SyntaxError: Unexpected token '*'
at ExecutionContext._evaluateInContext (~/lighthouse/core/gather/driver/execution-context.js:140:31)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async ExecutionContext.evaluateAsync (~/lighthouse/core/gather/driver/execution-context.js:172:14)
at async LinkElements._getArtifact (~/lighthouse/core/gather/gatherers/link-elements.js:183:17)
at async ~/lighthouse/core/gather/runner-helpers.js:101:24
at async collectPhaseArtifacts (~/lighthouse/core/gather/runner-helpers.js:118:5)
at async _computeNavigationResult (~/lighthouse/core/gather/navigation-runner.js:200:5)
at async _navigations (~/lighthouse/core/gather/navigation-runner.js:297:30)
at async ~/lighthouse/core/gather/navigation-runner.js:370:27
at async Runner.gather (~/lighthouse/core/runner.js:218:21)'
vs the old new Error(`Evaluation exception: ${errorMessage}`)'s stack:
Error: Evaluation exception: SyntaxError: Unexpected token '*'
at ExecutionContext._evaluateInContext (~/lighthouse/core/gather/driver/execution-context.js:140:31)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async ExecutionContext.evaluateAsync (~/lighthouse/core/gather/driver/execution-context.js:172:14)
at async LinkElements._getArtifact (~/lighthouse/core/gather/gatherers/link-elements.js:183:17)
at async ~/lighthouse/core/gather/runner-helpers.js:101:24
at async collectPhaseArtifacts (~/lighthouse/core/gather/runner-helpers.js:118:5)
at async _computeNavigationResult (~/lighthouse/core/gather/navigation-runner.js:200:5)
at async _navigations (~/lighthouse/core/gather/navigation-runner.js:297:30)
at async ~/lighthouse/core/gather/navigation-runner.js:370:27
at async Runner.gather (~/lighthouse/core/runner.js:218:21)
looks exactly the same except the new version has more non-stack stuff at the top
| const evaluationError = new Error(`Runtime.evaluate exception: ${message}`); | ||
| if (ex.exception?.description && ex.stackTrace) { | ||
| // The description contains the stack trace formatted as expected, if present. | ||
| evaluationError.stack = ex.exception.description; |
There was a problem hiding this comment.
I think that's what @adamraine meant, otherwise you lose the evaluate() call stack on the LH side. In theory not a huge deal: if you're getting an error this early, you were probably just editing the js and know where the call was coming from, but it can be an issue for evaluate calls originating in many places
| */ | ||
| constructor(errorDefinition, properties) { | ||
| super(errorDefinition.code); | ||
| constructor(errorDefinition, properties, cause) { |
There was a problem hiding this comment.
copying the Error options param might be better instead of just having a cause param, both to match the parent's style and so it's a named argument at call sites
There was a problem hiding this comment.
done - but now need to bump tsc to get ErrorOptions to be unknown instead of Error.
| auditResult.description = '**Excluded from diff**'; | ||
| } | ||
| if (auditResult.errorStack) { | ||
| auditResult.errorStack = auditResult.errorStack.replaceAll(baseCallFrameUrl.href, ''); |
There was a problem hiding this comment.
It's still pretty of annoying to update sample LHRs for line number changes in any of the files in an error stack. Could do like errorStack.replaceAll(/\([^)]+\)/g, '()') but maybe we should snip it completely
5a3adcf to
1210b54
Compare
1210b54 to
aa0c628
Compare
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
aa0c628 to
d041926
Compare
| const evaluationError = new Error(`Runtime.evaluate exception: ${message}`); | ||
| if (ex.exception?.description && ex.stackTrace) { | ||
| // The description contains the stack trace formatted as expected, if present. | ||
| evaluationError.stack = ex.exception.description; |
There was a problem hiding this comment.
did something change? Here's the error stack I get on hreflang if I make an error in LinkElements:
errorMessage: Required LinkElements gatherer encountered an error: getElementsInDocumentz is not defined
errorStack: Error: LighthouseError: ERRORED_REQUIRED_ARTIFACT
at Runner._runAudit (~/lighthouse/core/runner.js:431:25)
at Runner._runAudits (~/lighthouse/core/runner.js:370:40)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Runner.audit (~/lighthouse/core/runner.js:62:32)
at async runLighthouse (~/lighthouse/cli/run.js:250:8)
at async ~/lighthouse/cli/index.js:10:1
at <anonymous>:1:1
This adds an optional
errorStackto an error'd audit product.Also included is some better handling for our
.evaluatefunction for when there is a syntax error.example: