fix(firebase_crashlytics): Include obfuscated stack traces by untp · Pull Request #4407 · firebase/flutterfire

untp · 2020-12-16T11:54:47Z

Description

No stack traces appear for crashes that from obfuscated dart code. Because current implementation can't parse obfuscated stack traces. This PR implementation sends unparsed stack traces if the stack trace is obfuscated.

Deobfuscating can be done like this.

Problems

This implementation removes warning line, PID and instructions lines, abs addresses. Because these variables are different between sessions, so the same error will be grouped in Crashlytics Console. (These variables are not important, only _kDartIsolateSnapshotInstructions+0x lines are needed for deobfuscating)
Crashlytics Console appends (.java) and prepends . every line. Developers need to trim these lines manually.

Screenshots from Crashlytics Console

If warning, pid, instructions lines not removed: (all errors are grouped)

If warning, pid, instructions lines are removed, but abs is not removed: (These issues are same error with different sessions. Their virt addresses are same, but abs addresses are different. Abs addresses are must be removed for grouping same errors.)

Related Issues

Fixes #1150
Fixes #2644

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (flutter analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
I signed the CLA.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

…lytics Console

Ehesp · 2020-12-17T11:10:35Z

Wow nice one for this PR! Will get it reviewed.

talent-apps · 2020-12-22T21:31:06Z

@Ehesp Please prioritize this issue.
It is crucial for Flutter production apps.
🙏

talent-apps · 2020-12-24T17:30:07Z

@untp
We tested the PR and we successfully managed to deobfuscate crashes from the Crashlytics reports.
One question remains: each report on the dashboard contains a reason field which is also obfuscated.
Example: Error thrown Instance of 'Ax'.
Is there a way to deobfuscate it? It could be very helpful for the analysis.

talent-apps · 2020-12-25T16:49:57Z

@untp On iOS the stack trace lines have the following format:
#00 abs 000000011615a293 _kDartIsolateSnapshotInstructions+0x150293
The current regexp doesn't pick those lines. Maybe we need a different regexp for iOS?

untp · 2020-12-25T19:54:49Z

@talent-apps awesome, thanks for testing. I forgot to test on iOS. I fixed it with the above commit. If you catch any bugs, I would appreciate it if you let me know.

talent-apps · 2020-12-26T08:01:09Z

@untp When testing the new RegExp, the app force quits upon dart non-fatal crash and we don't see any reports on the dashboard.
Any idea why?

…able

untp · 2020-12-26T12:29:14Z

@talent-apps thanks, I fixed it. The problem was file field of stack trace lines is not nullable on iOS.

talent-apps · 2020-12-26T16:09:04Z

@untp We can confirm that the deobfuscation works on both platforms now. 🥳
Thanks for this important contribution.
It may take months until this PR is reviewed since this plugin is undermaintained.
We will not wait for an official release and we will start using it in production.

dayron9110 · 2021-03-02T12:50:37Z

Any updates?

Vitor-Bukovitz · 2021-03-02T16:24:56Z

@Ehesp & @Salakar can you guys take a look at this PR? This may solve the issue #80

envercc · 2021-03-22T02:51:01Z

This one would be really helpful, please take a look into it!

untp · 2021-03-23T18:30:45Z

@rrousselGit @Salakar @Ehesp
Sorry for spamming, but this PR is 3 months old, and it fixes a very important issue (#2644) which has impact: crowd tag. This issue is in the 1st page if issues are sorted to most reactions 👍. This issue is fixed with this PR, and it only needs to be reviewed.

Can you review this PR please?

packages/firebase_crashlytics/firebase_crashlytics/test/firebase_crashlytics_test.dart

rrousselGit

LGTM

But I wonder why those (.java) are added. Thought @russellwheatley?

russellwheatley

@rrousselGit I don't know why the Crashyltics backend appends .java to each line, my best guess would be to indicate it came from an android app.

PR LGTM once Remi's test issues have been addressed.

untp · 2021-04-08T09:26:16Z

The file field is an empty string, so it appears as .java. I tried with null, but it didn't work. This issue can only be fixed on the backend. Crashlytics backend should not append .java if file field is empty. Also on iOS it prepends 0 ??? 0x0.

untp · 2021-04-08T11:47:13Z

I marked this as draft. I tested now, it outputs the warning line. This shouldn't happen.

Show image

EDIT: I found the issue. Obfuscated stack traces are now including this line after pid: ... tid: ... line:
build_id: '77d1b910140a2d66b93594aea59469cf'
Trace API thinks this is a friendly trace. Friendly Trace's regex:
RegExp(r'^[^\s<][^\s]*( \d+(:\d+)?)?[ \t]+[^\s]+$', multiLine: true);
We need to force Trace API to parse VM stack trace.

So I will change Trace.from to Trace.parseVM.

untp · 2021-04-08T16:00:39Z

Now it works both obfuscated and non-obfuscated.

russellwheatley

LGTM. What do you think, @rrousselGit?

rrousselGit

LGTM

fix(firebase_crashlytics): Include obfuscated stack traces

753fd9f

google-cla bot added the cla: yes label Dec 16, 2020

This comment has been minimized.

Sign in to view

fix(firebase_crashlytics): clean getStackTraceElements implementation

7c1069d

This comment has been minimized.

Sign in to view

fix(firebase_crashlytics): same exceptions should be grouped in Crash…

51c1ceb

…lytics Console

This comment has been minimized.

Sign in to view

fix(firebase_crashlytics): fix ios obfuscated stack trace

6894efd

fix(firebase_crashlytics): file field of stack trace line is not null…

e7baa8f

…able

fix(firebase_crashlytics): fix tests

51359a1

Merge branch 'master' into obfuscated-stack-traces

0636c97

deepak786 mentioned this pull request Jan 18, 2021

[firebase_crashlytics] No stack traces for issues on Crashlytics dashboard #2644

Closed

untp added 2 commits January 22, 2021 16:01

Merge branch 'master' into obfuscated-stack-traces

105fcf0

fix(firebase_crashlytics): Migrate to NNBD

376b395

rrousselGit self-assigned this Apr 1, 2021

Salakar requested review from Ehesp, rrousselGit and russellwheatley April 7, 2021 08:47

Salakar requested a review from Ehesp April 7, 2021 08:48

rrousselGit reviewed Apr 8, 2021

View reviewed changes

packages/firebase_crashlytics/firebase_crashlytics/test/firebase_crashlytics_test.dart Outdated Show resolved Hide resolved

rrousselGit reviewed Apr 8, 2021

View reviewed changes

packages/firebase_crashlytics/firebase_crashlytics/test/firebase_crashlytics_test.dart Show resolved Hide resolved

rrousselGit approved these changes Apr 8, 2021

View reviewed changes

russellwheatley approved these changes Apr 8, 2021

View reviewed changes

Remove unnecessary test and edit test descriptions

d9943a6

untp marked this pull request as draft April 8, 2021 11:45

untp added 2 commits April 8, 2021 17:18

Force parsing stack traces as VM traces

c46a2d0

Update tests to new obfuscated stack trace format

e9df460

untp marked this pull request as ready for review April 8, 2021 16:00

untp requested review from rrousselGit and russellwheatley April 8, 2021 16:00

russellwheatley approved these changes Apr 19, 2021

View reviewed changes

rrousselGit approved these changes Apr 19, 2021

View reviewed changes

Salakar merged commit 6ff4bb9 into firebase:master Apr 19, 2021

untp mentioned this pull request Apr 19, 2021

🐛 [firebase_crashlytics] - Flutter/Android Obfuscated Trace #5815

Closed

untp deleted the obfuscated-stack-traces branch April 19, 2021 20:24

firebase locked and limited conversation to collaborators May 20, 2021

Conversation

untp commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problems

Screenshots from Crashlytics Console

Related Issues

Checklist

Breaking Change

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Ehesp commented Dec 17, 2020

Uh oh!

talent-apps commented Dec 22, 2020

Uh oh!

talent-apps commented Dec 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

talent-apps commented Dec 25, 2020

Uh oh!

untp commented Dec 25, 2020

Uh oh!

talent-apps commented Dec 26, 2020

Uh oh!

untp commented Dec 26, 2020

Uh oh!

talent-apps commented Dec 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dayron9110 commented Mar 2, 2021

Uh oh!

Vitor-Bukovitz commented Mar 2, 2021

Uh oh!

envercc commented Mar 22, 2021

Uh oh!

untp commented Mar 23, 2021

Uh oh!

Uh oh!

Uh oh!

rrousselGit left a comment

Choose a reason for hiding this comment

Uh oh!

russellwheatley left a comment

Choose a reason for hiding this comment

Uh oh!

untp commented Apr 8, 2021

Uh oh!

untp commented Apr 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

untp commented Apr 8, 2021

Uh oh!

russellwheatley left a comment

Choose a reason for hiding this comment

Uh oh!

rrousselGit left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

untp commented Dec 16, 2020 •

edited

Loading

talent-apps commented Dec 24, 2020 •

edited

Loading

talent-apps commented Dec 26, 2020 •

edited

Loading

untp commented Apr 8, 2021 •

edited

Loading