core: smooth rough edges of pageLoadError display and reporting #6083
Conversation
brendankenny
requested review from
patrickhulce and
paulirish
as code owners
This comment has been minimized.
This comment has been minimized.
|
I will fight you, devtools-bot |
|
Love it. No more red text. |
| @@ -301,9 +301,7 @@ class Runner { | |||
| static getArtifactRuntimeError(artifacts) { | |||
| for (const possibleErrorArtifact of Object.values(artifacts)) { | |||
| if (possibleErrorArtifact instanceof LHError && possibleErrorArtifact.lhrRuntimeError) { | |||
| const errorMessage = possibleErrorArtifact.friendlyMessage ? | |||
| `${possibleErrorArtifact.friendlyMessage} (${possibleErrorArtifact.message})` : | |||
There was a problem hiding this comment.
this concatenation is now directly handled by gather runner now, right? was that the only case we fell into this path?
There was a problem hiding this comment.
Yeah, in gather runner. And since this was only introduced in #6051, there's nothing depending on it (only lr uses it, and only for human debugging; it's never shown to users).
message here is the error code, so it seemed redundant to keep it when the code is sitting right next to it on the object. I can add it back if we want to match the other place(s?) we put together an error string out of an lherror
There was a problem hiding this comment.
Nah, sounds good as-is 👍
| } else if (mainRecord.failed) { | ||
| errorCode = LHError.errors.FAILED_DOCUMENT_REQUEST; | ||
| errorReason = mainRecord.localizedFailDescription; | ||
| errorDef = {...LHError.errors.FAILED_DOCUMENT_REQUEST}; |
There was a problem hiding this comment.
why so fancy?
easier than Object.assign()? Dunno :) Just needed a shallow copy so it's not modifying the original message
fixes #6071
runtimeErrorand individual audit errors now take care of warning the user in that case)friendlyMessagein HTML report's warning box (runWarnings)bonus: no longer have a big red
GatherRunner:error FAILED_DOCUMENT_REQUESTerror log on the pageLoadError we expect in the offline pass when a page doesn't support offline :) @exterkamp