core: avoid fatal errors when collecting base artifacts #13312
Conversation
so, this was actually intentional because the code to handle an This changed with fraggle rock which has generalized gatherer dependencies and so will handle error artifacts used by gatherers: lighthouse/lighthouse-core/fraggle-rock/gather/runner-helpers.js Lines 120 to 122 in 5430f02 Since this is classic code, the catch should probably just set defaults, though that's not quite the right thing to do for That said, the gatherer ( |
|
falling back to baseartifact-specific default states sgtm |
| baseArtifacts.InstallabilityErrors = { | ||
| errors: [ | ||
| { | ||
| errorId: 'protocol-timeout', |
There was a problem hiding this comment.
Protocol timeouts are probably the only errors we will encounter, but the message doesn't say anything about protocol timeouts. WDYT about a more generic name with lh- prefix like lh-collection-error?
There was a problem hiding this comment.
The error code is exactly correct as to why this error would occur (what else would cause an error to be thrown here)? The user doesn't need to know anything about the details, "protocol" wouldn't mean anything to them.
There was a problem hiding this comment.
I was thinking about future code in this catch block that could emit a different error. Since we do this for any generic error, seems like it should have a generic name 🤷 . It's a nit though, so we can land without.
5 participants
Gatherers for our non-base artifacts recover from errors by resolving the artifact value to an Error object, and marching on, allowing the audit runner to generate an error encountering an error'd artifact. But, for our base artifacts in the legacy gatherer, we don't handle error cases. So if a protocol method errors or times out, the result is a fatal error and the run ends with no auditing.
This PR simply adds a try/catch around the base artifacts that communicate use the protocol. We are seeing a lot of timeouts w/ the installability errors artifact specifically, so this should provide better handling in that case (we still need to discover the root cause there, but at least users will get a report but with error'd PWA category).
see #13273 (comment)
New result in DevTools for https://www.enricofromrome.eu/edb6/ (which would previously hang forever):