1563305 - Make ProfilerGetSymbols.jsm throw more detailed errors if symbolication goes wrong for any reason

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P3)

Description

[Markus Stange [:mstange]]

We have code in ProfilerGetSymbols.jsm and its dependencies that takes a path to a binary on the local file system, and an ID, and returns a symbol table for that binary.

This code can fail, for a variety of reasons:

• The file might not be present at the given path.
• The file contents could not be loaded into memory.
• The file contents are not recognizable as any supported binary type.
• The given breakpad ID was malformed.
• The ID computed for the binary at the given location does not match the given breakpad ID, for example because the binary was updated as the result of a more recent compile.
• There was a parse error.
• Some other error occurred.

Today, all those failures are reported as the same type of error. It is very hard to find out which of these things actually went wrong. On the rust side, all of those failures are conflated in the None value of on Option and then returned as false through the FFI boundary.

We should change this code to use proper rust Result types with different error kinds, and then expose them in some way to JS through the rust/WASM/JS boundary.

Comment 1

[Christina Cheung]

Attachment: Bug 1563305: Make ProfilerGetSymbols.jsm throw more detailed errors if symbolication goes wrong for any reason

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1563305 - Make error objects propagate correctly from the worker into the profiler page. r=julienw

Depends on D38718

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:ccheung256, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 5

[Markus Stange [:mstange]]

I will take over the rest of the work here at some point.

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1563305 - Propagate more worker errors. r=julienw

This adds onerror and onmessageerror handlers.
The former is called if there's a syntax error in the worker script, for example.
Without this handler, such mistakes can easily fail silently and go unnoticed.
The latter is supposed to be called when a message is received that cannot be
deserialized. I'm not sure how to get into that state, but it seems like a good
idea to add the handler anyway.

There are some worker errors that these two handlers do not catch. For example,
if the worker tries to send a message that is not Structured Cloneable, neither
of these handlers is called; instead, the postMessage call within the worker
throws an error, and that error needs to be manually propagated from the worker
to the origin thread, via postMessage. And if such an error is encountered
within the error propagation code itself, we're out of luck.

Moreover, somewhat unrelatedly: WorkerGlobalScope.onerror is not called for
unhandled promise rejections. So if we throw an error within an async function
inside the worker, we can only handle that error if it happens within a
try {} catch {} block.

Depends on D44402

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd6c19a8a055
Make ProfilerGetSymbols.jsm throw more detailed errors if symbolication goes wrong for any reason r=mstange
https://hg.mozilla.org/integration/autoland/rev/d7d489555f5f
Make error objects propagate correctly from the worker into the profiler page. r=julienw
https://hg.mozilla.org/integration/autoland/rev/7bf3e2fd554c
Propagate more worker errors. r=julienw

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/fd6c19a8a055
https://hg.mozilla.org/mozilla-central/rev/d7d489555f5f
https://hg.mozilla.org/mozilla-central/rev/7bf3e2fd554c