evaluateAsync doesn't return rejected promises properly

[patrickhulce]

Partially discussed in #965

Repro steps:

1. Add a bug in gatherers/dobetterweb/links-blocking-first-paint.js or any other browser eval'd function that should cause the promise to be rejected
2. Run lighthouse with dbw audits enabled.
3. Observe that the debug string is tags.reduce is not a function or some similar unhelpful message masking the underlying issue.

Issue lies in JSON serialization of the result and the rejected promise being converted to an object like below instead of a rejected promise see driver.js.

{
  "result": {
    "type": "object",
    "value": {}
  },
  "exceptionDetails": {
    "exceptionId": 1,
    "text": "Uncaught (in promise)",
    "lineNumber": 0,
    "columnNumber": 0,
    "stackTrace": {
      "callFrames": []
    },
    "exception": {
      "type": "object",
      "value": {}
    }
  }
}

[brendankenny]

If we fix this we need to be especially sure it's fixed in both CLI and extension.

The unhelpful promise rejection info is really unfortunate. Looks like it may be a devtools bug? It would be nice to have error message, stack trace, etc

[brendankenny]

In #965 the weird result.wasThrown check in the extension connection came up.

I can only find wasThrown in one place in the protocol, when fetching the properties of an object, and that result doesn't have an exceptionDetails property, so it's not entirely clear if that's ever actually hit in the extension codepath. Looking at the git blame for it, the reason it was added isn't abundantly clear (and we no longer do what we were doing there), so it's possible it's a red herring and just needs to be removed.

[brendankenny]

so it's possible it's a red herring and just needs to be removed

this is being removed in #977

[brendankenny]

@patrickhulce this was fixed for evaluateAsync in #965, right?

We could close or we could expand the issue scope to all the protocol methods we currently use that return an object with possible exceptionDetails

[patrickhulce]

Yeah that was pretty much just a stop gap since anything that's an actual Error object would be totally empty but in that case we were explicitly rejecting with just a string, I think expanding the scope to properly handle those errors makes sense

[brendankenny]

#1037 has a nice solution to this