fix: exclude PageSpeed Insights API errors from retries.

[adamsilverstein]

When retries are enabled in the client, the Runner class by default retries requests when the API returns a 500 error code. Unfortunately, the PageSpeed Insights API returns 5XX's for errors that should really be 4XX (specifically in Lighthouse for the errors ERRORED_DOCUMENT_REQUEST, FAILED_DOCUMENT_REQUEST). So 500 errors from this API shouldn't be retried, they will only increase load and fail again.

These errors are non-retryable user defined errors: for example requesting data for a url that doesn't go anywhere. This PR changes the default client behavior to avoid retrying requests the the reason lighthouseError, preventing unnecessary retries.

Steps to reproduce

1. Visit the PageSpeed Insights API explorer page
2. Enter a domain that does non exist in the URL field, for example https://notareal-websiteno.com, and run the report.
3. Note the returned error response is a 500 error.

Additional Context

We recently added this change the the defaults used by Site Kit which relies on this library. Since we found this condition important enough to explicitly exclude, I thought it might be worth contributing upstream, although I'm not sure you want to add these types of API specific exceptions.

[bshaffer]

@adamsilverstein thank you for this change!

I am not sure the implications of the reason lighthouseError, and if errors with this reason should never be retried. Do you know of any documentation for what lighthouseError means? If it's not always a result of a bad request, e.g. 400, then we may need something more robust.

[bshaffer]

Looking more into it, Lighthouse is our tool for analyzing / auditing the webpages. When this downstream library throws an error, the wrapping API throws a 500 and sets the reason as "lighthouseError".

Unfortunately, looking at possible error responses, some of them do correspond to 500 errors that should be retried. For example:

Lighthouse returned error: INTERNAL: APP::1: Lost browser connection.

In these cases, it would be best if the libraries retried this request. So is it better to retry all requests so that the ones that need retrying get it, or to not retry any requests, to prevent unnecessary retries? A third option would be to allow for certain "error" values to be configured.

I think the error above is not a common error, and results from a bug that shouldn't be retried anyway. So I approve this change.

[adamsilverstein]

@bshaffer Thank you for reviewing and approving.