Skip to content

Crashlytics GDT fill in Exception part of Proto #4894

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

Merged
merged 5 commits into from
Feb 13, 2020

Conversation

samedson
Copy link
Contributor

@samedson samedson commented Feb 12, 2020

  • Sets the fields in the Exception part of the app execution.
  • Also fixes the analysis issue with signalProto

Does not fill in symbolicated files or importance for symbolicated.

@samedson samedson changed the title Crashlytics firelog exception frames Crashlytics Firelog fill in Exception part of Proto Feb 12, 2020
@@ -26,6 +26,8 @@
#import <nanopb/pb_decode.h>
#import <nanopb/pb_encode.h>

const NSUInteger IMPORTANCE_FOR_EXCEPTION = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming for this may be confusing since the importance for exceptions be non-zero. Take a look at Protobuf.scala:384.

Maybe this should be DEFAULT_IMPORTANCE_FOR_EXCEPTION?

@@ -26,6 +26,8 @@
#import <nanopb/pb_decode.h>
#import <nanopb/pb_encode.h>

const NSUInteger IMPORTANCE_FOR_EXCEPTION = 0;
Copy link
Contributor

@jasonhu-g jasonhu-g Feb 12, 2020

Choose a reason for hiding this comment

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

In FIRCLSRecordThread.m:21, we define IMPORTANCE_IN_CRASHED_THREAD. Does it make more sense to have this const be defined FIRCLSRecordFrame? FIRCLSRecordFrame could also expose an importance property to abstract the logic from this adapter class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that probably makes more sense to organize it all there

@samedson samedson changed the title Crashlytics Firelog fill in Exception part of Proto Crashlytics GDT fill in Exception part of Proto Feb 13, 2020
@samedson samedson merged commit e64f1a9 into crashlytics-firelog Feb 13, 2020
@samedson samedson deleted the crashlytics-firelog-exception-frames branch February 13, 2020 15:37
@firebase firebase locked and limited conversation to collaborators Mar 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants