core: avoid fatal errors when collecting base artifacts

[connorjclark]

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):

[brendankenny]

But, for our base artifacts in the legacy gatherer, we don't handle error cases

so, this was actually intentional because the code to handle an Error artifact was in runner just for audits, but baseArtifacts could be used as inputs by gatherers. They have no protection from an Error instead of the base artifact they expect. Instead, base artifacts were supposed to be individually robust and generate a default value (null or whatever) in the face of errors, though it looks like they never handled protocol timeouts all that well.

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

	if (dependencyArtifact instanceof Error) {
	throw createDependencyError(dependency, dependencyArtifact);
	}

Since this is classic code, the catch should probably just set defaults, though that's not quite the right thing to do for InstallabilityErrors since no errors in the artifact would be equivalent to installation with no errors :/

That said, the gatherer (start-url) that depended on accessing WebAppManifest and InstallabilityErrors is long gone. Those two could be just regular gatherers now which would take care of the problem and support proper error propagation (and the FR work already made them into gatherers, they just classic-mode support). Stacks is a little different because it's more just that we want it to run no matter what, but there's less of a problem with defaulting to [] in case of an error.

[connorjclark]

falling back to baseartifact-specific default states sgtm

[adamraine]

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?

[connorjclark]

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.

[adamraine]

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.