Skip to content

Conversation

@romtsn
Copy link
Member

@romtsn romtsn commented Dec 4, 2025

Closes #63

  • Store rewrite rules in ProguardCache format
  • Decode rewrite rules and use them for symbolication
  • Additionally parse the exception descriptor
  • Make new methods public to be consumed in symbolicator later on

See linked issue for more details.

Copy link
Contributor

@loewenheim loewenheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to consider how you're going to use this from Symbolicator, since you won't be able to just use remap_stacktrace. My suggestion would be to move some of the logic into remap_frame.

src/cache/mod.rs Outdated
Comment on lines 714 to 717
}

next_frame_can_rewrite = false;
current_exception_descriptor = None;

This comment was marked as outdated.

Comment on lines +249 to +251
raw::REWRITE_CONDITION_UNKNOWN => {
if let Ok(value) = self.read_string(component.value) {
conditions.push(RewriteCondition::Unknown(value));

This comment was marked as outdated.

@romtsn
Copy link
Member Author

romtsn commented Dec 12, 2025

You also need to consider how you're going to use this from Symbolicator, since you won't be able to just use remap_stacktrace. My suggestion would be to move some of the logic into remap_frame.

@loewenheim I have attempted to consolidate the logic under remap_frame and extend the custom iterator to account for skip functionality. Let me know if this is overly-complicated or looks good -- I can probably apply the same for the Mapper (although not sure if it's necessary since it's not being reused in symbolicator like the Cache?). Here's the relevant symbolicator PR with the caller side usage getsentry/symbolicator#1845

@loewenheim
Copy link
Contributor

@romtsn The logic is good. However, I would suggest leaving remap_frame the way it was and adding your version as a new method. That way, this only requires a minor update. I agree with you that it should be fine to not do this (now) for the mapper.

@romtsn
Copy link
Member Author

romtsn commented Dec 17, 2025

However, I would suggest leaving remap_frame the way it was and adding your version as a new method. That way, this only requires a minor update.

thanks, this makes sense! I will move it over to a new method

Base automatically changed from rz/feat/rewrite-frames-support to master December 17, 2025 13:06
@romtsn romtsn merged commit 9f12864 into master Dec 18, 2025
8 checks passed
@romtsn romtsn deleted the rz/feat/rewrite-frames-support-cache branch December 18, 2025 14:15
romtsn added a commit to getsentry/symbolicator that referenced this pull request Dec 19, 2025
Attempts to reuse `remap_frame` from ProguardCache to delegate as much
logic to it as possible and keep it lean on the caller side.

Additionally, had to extend the `JvmStacktrace` interface, because we
now need the `JvmException` associated with the stacktrace to correctly
apply rewrite rules.

Depends on getsentry/rust-proguard#67
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support rewriteFrame annotation

3 participants