-
-
Notifications
You must be signed in to change notification settings - Fork 338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(event): Message event contains stacktrace if attachStacktrace
o…
#2577
Conversation
@krystofwoldrich I lack context here, didn't agree on fixing this on JS? getsentry/sentry-javascript#6038 |
Yes, we will fix the JS part, but besides that, there was an option missing in RN. But after adding this option the stack trace is still missing when |
Gotcha, I'd expect this change to be part of #2461 and not #2131 |
@marandaneto I can split it into two PRs, first "enable stack trace by default" and the second this one. When we use the Only when calling the This is some extreme magic, but at least we are not blocked by it when calling the method on the client directly. The issue happens in function eventFromString(
stackParser,
input,
syntheticException,
attachStacktrace,
) {
const event = {
message: input,
};
console.log('syntheticException', syntheticException);
if (attachStacktrace && syntheticException) {
const frames = parseStackFrames(stackParser, syntheticException); # frames.length > 0
if (frames.length) {
console.log('frames', frames);
console.log('stacktrace', {stacktrace: { frames }});
console.log('values', [{ value: input, stacktrace: { frames } }]);
console.log('exception', {
values: [{ value: input, stacktrace: { frames } }],
});
console.log('event.exception before', event.exception); # till here everything works as expected, frames are present
event.exception = {
values: [{ value: input, stacktrace: { frames, my_frames: frames } }],
};
event.super_long_name_12345678 = {
values: [{ value: input, stacktrace: { frames } }],
};
console.log('event.exception', event.exception); # event.exception.values.stacktrace.frames = []
# event.exception.values.stacktrace.my_frames = [the correct frames]
# event.super_long_name_12345678.values.stacktrace.frames = [the correct frames]
}
}
console.log('event_final', event);
return event;
} |
This is the weirdest bug I've seen in a loooong time. As discussed with @krystofwoldrich we probably want to address the bug like so:
|
Ok, that makes sense, I just lacked the context around this and the ingestion. |
…trace` option is set" This reverts commit 65fec08.
Thank you @AbhiPrasad and @marandaneto So the bug is caused by So I think this can be reviewed and merged since in production the stack frames will be sent correctly. For #2461 I will create a new PR that will merge to |
This still depends on the JS SDK bump, after the other PR, right? |
The JS SDK bump will move the stack traces from exceptions to threads, but from |
📢 Type of change
📜 Description
Theoretically only passing the option to the browser client should solve the problem. But for some unexplainable reason, the frames are always an empty array, if the synthetic error is not passed from the RN JS code.
Code from JS SDK.
https://github.com/getsentry/sentry-javascript/blob/e1d43ebb34086605aefb9243fce03fc97425b4be/packages/browser/src/eventbuilder.ts#L276
💡 Motivation and Context
Fixes: #2131
💚 How did you test it?
📝 Checklist
🔮 Next steps