fix: allow optional abs_path and lineno for JsFrame

[samuelmaddock]

fixes #1580

It's possible for V8 to format a JS frame location to only contain <anonymous> (see AppendFileLocation). In such cases, JSON deserialization should pass, but symbolification should fail.

Here's an example call stack this addresses:

Error: Example unresponsive call stack
    at Object.hasOwnProperty (<anonymous>)

Also, this is the first Rust code I've written so please lmk what can be improved!

[codecov]

Codecov Report

Attention: Patch coverage is 68.85246% with 19 lines in your changes missing coverage. Please review.

Project coverage is 73.59%. Comparing base (5a85fd7) to head (32ce6d9).
Report is 205 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1581      +/-   ##
==========================================
- Coverage   76.09%   73.59%   -2.50%     
==========================================
  Files         101      105       +4     
  Lines       15262    15912     +650     
==========================================
+ Hits        11613    11710      +97     
- Misses       3649     4202     +553     

[Dav1dde]

Just left some small Rust nits, otherwise looks good to me, but I want to wait on @loewenheim for a review, since I can't really judge the implications of the change fully.

Also would be great if you can add a test, I think symbolicator-js/tests/integration/sourcemap.rs should be the right place and pretty low effort to add.

[Dav1dde]

Can use abs_path.to_owned() here now, I think, since we already checked for Some before.

Suggested change

	.unwrap_or_else(|| raw_frame.abs_path.clone().unwrap_or_default());
	.unwrap_or_else(|| abs_path.to_owned());

[samuelmaddock]

I had to revert this in 12d1d5e to fix a compilation error

error[E0502]: cannot borrow `*raw_frame` as mutable because it is also borrowed as immutable
   --> crates/symbolicator-js/src/symbolication.rs:145:17
    |
105 |   ...ath = raw_frame
    |  __________-
106 | | ...path
    | |_______- immutable borrow occurs here
...
145 |   ...   apply_source_context(raw_frame, minified_source.contents(...
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
...
163 |   ...ap_or_else(|| abs_path.to_owned());
    |                    -------- immutable borrow later captured here by closure

[loewenheim]

If you clone abs_path at the top of the function this just becomes

Suggested change

	.unwrap_or_else(|| raw_frame.abs_path.clone().unwrap_or_default());
	.unwrap_or(abs_path.clone());

[samuelmaddock]

@Dav1dde I've added a couple snapshot tests in 41144b1

[samuelmaddock]

@Dav1dde @loewenheim Our product will start receiving a higher volume of these unsymbolicated call stacks next week. Is there a timeline we can follow for how soon an update to the symbolicator service might be deployed by Sentry?

[loewenheim]

@samuelmaddock I was under the impression that you wanted this because you are running Symbolicator yourself, did I misunderstand that? On Sentry's side nothing will immediately change even when this is deployed—we don't send frames without abs_path to Symbolicator; they are separated out and then merged back into the symbolicated stacktrace that comes back from Symbolicator.

[loewenheim]

If you clone abs_path at the top of the function this just becomes

Suggested change

	.unwrap_or_else(|| raw_frame.abs_path.clone().unwrap_or_default());
	.unwrap_or(abs_path.clone());

[samuelmaddock]

@samuelmaddock I was under the impression that you wanted this because you are running Symbolicator yourself, did I misunderstand that? On Sentry's side nothing will immediately change even when this is deployed—we don't send frames without abs_path to Symbolicator; they are separated out and then merged back into the symbolicated stacktrace that comes back from Symbolicator.

@loewenheim Maybe I'm misunderstanding the underlying problem in that case.

We have call stacks being sent to Sentry which are formatted like the example in the PR description. Our events within our non-self-hosted Sentry are not being symbolicated. I copied the exception.values[0].stacktrace from the event's JSON API response and found that a locally hosted symbolicator service refuses to symbolicate it with the error:

Failed to deserialize the JSON body into the target type: stacktraces[0].frames[0]: missing field `abs_path` at line 1 column 220

My hope was that by fixing this issue in the symbolicator service, it will also fix it in Sentry once the fix has been deployed.

It's possible we've made a configuration error which is preventing symbolication, but it has been working for other events. Is there a better place we could raise this issue?

[loewenheim]

Oh shoot, in that case there's been a miscommunication. No, SaaS is already robust against stack frames without abs_paths. Can you post a link to an event that isn't symbolicated as you would expect, or email it to me at [email protected]?