ref: Target upstream rust-minidump

[flub]

This adapts to the async API of rust-minidump but also does a few other things:

• Changes the FrameTrust mapping to our own struct so we can impl Default etc we need for it.
• Completely ignores registers for now, this is a regression

Apart from the obvious there are still a few test failures that I haven't gotten to the bottom off.

#skip-changelog

[Swatinem]

can you keep this? I consider this quite useful, I often need to switch to a local symbolic version.

[flub]

shouldn't this be done in a patch.crates-io section? Having that here makes sorting a lot harder :)

https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#working-with-an-unpublished-minor-version

[github-actions]

	Warnings
⚠️	Snapshot changes likely affect Sentry tests. If the Sentry-Symbolicator Integration Tests in CI are failing for your PR, please check the symbolicator test suite in Sentry and update snapshots as needed.

Instructions for snapshot changes

Sentry runs a symbolicator integration test suite located at tests/symbolicator/. Changes in this PR will likely result in snapshot diffs in Sentry, which will break the master branch and in-progress PRs.

Follow these steps to update snapshots in Sentry:

1. Check out latest Sentry master and enable the virtualenv.
2. Enable symbolicator (symbolicator: true) in sentry via ~/.sentry/config.yml.
3. Make sure your other devservices are running via sentry devservices up --exclude symbolicator. If
   they're already running, stop symbolicator with sentry devservices down symbolicator. You want to use your
   own development symbolicator to update the snapshots.
4. Run your development symbolicator on port 3021, or whatever port symbolicator is configured to use
   in ~/.sentry/config.yml.
5. Export SENTRY_SNAPSHOTS_WRITEBACK=1 to automatically update the existing snapshots with your new
   results and run symbolicator tests with pytest (pytest tests/symbolicator).
6. Review snapshot changes locally, then create a PR to Sentry.
7. Merge the Symbolicator PR, then merge the Sentry PR.

Generated by 🚫 dangerJS against a24ecc0

[codecov-commenter]

Codecov Report

Merging #673 (a24ecc0) into release/rust-minidump (f6a0e32) will decrease coverage by 0.13%.
The diff coverage is 7.54%.

@@                    Coverage Diff                    @@
##           release/rust-minidump     #673      +/-   ##
=========================================================
- Coverage                  71.38%   71.25%   -0.14%     
=========================================================
  Files                         48       48              
  Lines                      11038    11024      -14     
=========================================================
- Hits                        7880     7855      -25     
- Misses                      3158     3169      +11     

Impacted Files	Coverage Δ
crates/symbolicator/src/services/minidump.rs	93.12% <ø> (ø)
...olicator/src/services/symbolication/comparisons.rs	0.00% <ø> (ø)
crates/symbolicator/src/services/symbolication.rs	66.38% <2.43%> (-0.15%)	⬇️
crates/symbolicator/src/types/mod.rs	55.30% <25.00%> (-5.11%)	⬇️
crates/symbolicator/src/metrics.rs	52.05% <0.00%> (-1.37%)	⬇️
...es/symbolicator/src/services/download/locations.rs	80.54% <0.00%> (-0.55%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6a0e32...a24ecc0. Read the comment docs.

[relaxolotl]

thanks for fixing this up!

[relaxolotl]

this is for my own personal curiosity, but what was the answer to this question?

[flub]

code_id can't be empty because of the new API which guarantees to give you one. But, hum, good question. Maybe this isn't true because you can still create those to be empty. I'll do this as a followup up though because this could be messy.

[relaxolotl]

Mixed feelings seeing this change because I think it makes it more difficult to see the main operation being acted upon here (self.files.contains_key), but I don't think it's a dealbreaker. Looks like logically this is the same, so this is more of a readability nit than anything else.

[flub]

hum, i see what you mean though the previous code ended up calling .contains_key() with a nil DebugId (the _or_default() part) which is really what I'd like to avoid. Combinators like this are pretty standard rust style I think, what I like about this is there's only one exit point in the function, though your argument of making .contains_key() stand out also makes sense.

[relaxolotl]

Thanks for including the changes coming from my recent PR in rust-minidump! Left some nitpicky thoughts but overall I don't think any of them should block this, which removes one of the big barriers when it comes to testing rust-minidump changes.