Lighthouse: feature-request: add reason to runtime error output

[samfoot]

Feature request summary

When a FAILED_DOCUMENT_REQUEST error occurs the error message does not give the actual reason for this as it's not part of the stack trace for example:

Runtime error encountered: Your page failed to load. Verify that the URL is valid and re-run Lighthouse. LHError: FAILED_DOCUMENT_REQUEST at Function.getPageLoadError (/lighthouse/lighthouse-core/gather/gather-runner.js:168:21) at Function.afterPass (/lighthouse/lighthouse-core/gather/gather-runner.js:273:38) at process._tickCallback (internal/process/next_tick.js:68:7)

I am proposing to add the actual reason be displayed (if available) from the error, for example:

Runtime error encountered: Your page failed to load. Verify that the URL is valid and re-run Lighthouse. Reason: net::ERR_CERT_COMMON_NAME_INVALID LHError: FAILED_DOCUMENT_REQUEST at Function.getPageLoadError (/lighthouse/lighthouse-core/gather/gather-runner.js:168:21) at Function.afterPass (/lighthouse/lighthouse-core/gather/gather-runner.js:273:38) at process._tickCallback (internal/process/next_tick.js:68:7)

I am willing to do the work to add this.

What is the motivation or use case for changing this?

Being able to determine what the actual error that caused the page to fail to load would better help me explain to my users why the audit failed.

How is this beneficial to Ligthhouse?

Better diagnostics for developers that use the CLI to programatically audit.

[devtools-bot]

Thanks chief! Appreciate you filing this bug. :clap:

This is a known issue, most well described in #2784. So, we'll automatically close this as a duplicate.

_However_, if you believe your bug is different than the cases described there, please comment here with "necessarily-wide-alpaca" and I'll reopen this bug. :robot: Beep beep boop.

[paulirish]

@samfoot yeah this sounds great. Happy for you to take this.

[paulirish]

@patrickhulce do you remember why we moved the errorReason from the Error's message into a property?

@samfoot At the very least showRuntimeError in the CLI could also log error.reason. But I also think it's reasonable to change the message based on the errorReason. Let's see what patrick sez..

[patrickhulce]

Oh yeah that was mostly for Sentry coalescing on the error message. In most of the cases we did this I think it was a win to not give so much debugging info to users who are confused, but in this case I agree it is a bit of a loss. I think it'd make sense to show the real reason here too 👍

Maybe we could do a transform of errors before sending them into the LHR? i.e. if the error has a .reason property then make it the message and stick the message into parens or something?

[brendankenny]

fixed in #6083, though the code currently doing this (using localization replacement) superseded that in #6457