Skip to content

Conversation

@konraddysput
Copy link
Collaborator

Why

As a user I would like to upload attachments when:

  • ANR,
  • Crash,
  • Exception,
  • Unhandled exception,
    happened. With attachment support, we can have more insights about a game state when the specific situation in my game occurred.

This pull request allows our users to set up attachment support for managed and native reports. To achieve that users can initialize Backtrace Client integration with the list of paths to attachments that they would like to send to us. Attachments will be included in every Backtrace report. In addition to that on iOS when OOM, Crash occurred, we're generating crash/oom attributes but also we store attachments. In the next session when we want to send attachments to Backtrace, old attachments and new attachments will be available in the OOM/Crash report.

Limitations

  • We can initialize native attachments only on game startup - unfortunately, we can't add more attachments later.
  • User is responsible for attachment size - application might crash because of attachment size - for example when user wants to upload 5GB of data to Backtrace as an attachment.

Updates:

This diff updates the backtrace-android native library available in unity and the backtrace-cocoa (objective-c) library.

Testing strategy

In all my tests I used:

  • single file (.txt)
  • multiple files (.txt and .jpg)
  • files that don't exist with files that exist on the hard drive

Native test

iOS

Attachments are available on app crash

  1. Set paths to attachments
  2. Crash app
  3. Relaunch Game
  4. Check the Backtrace report in Backtrace and validate if attachments from the previous session and current session are available.

OOM reports contain attachment paths

  1. Set paths to attachments
  2. Simulate memory warning
  3. Crash app via OOM script OR by using special workarounds.
  4. Check if the OOM report has attachments from the previous session and from the current session.

ANR reports contain attachment paths

  1. Set paths to attachments
  2. Generate ANR on the managed layer,
  3. Check if the ANR report has attachments set by the user in the current session

Android

Generate android Crash

  1. Set paths to attachments
  2. Crash app
  3. Check the Backtrace report in Backtrace and validate if attachments are available.

ANR reports contain attachment paths

  1. Set paths to attachments
  2. Generate ANR on the managed layer,
  3. Check if the ANR report has attachments set by the user in the current session

Managed tests

Send exception/unhandled exception/message and see if attachments are available

  1. Set the path to the attachment
  2. Generate an exception/unhandled exception/message
  3. Check the Backtrace report in Backtrace to see if

@konraddysput konraddysput self-assigned this Mar 11, 2021
@konraddysput konraddysput added the enhancement New feature or request label Mar 11, 2021
Copy link

@rqbacktrace rqbacktrace left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

Copy link
Contributor

@vlussenburg vlussenburg left a comment

Choose a reason for hiding this comment

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

The .so files look empty (0 bytes) in the diff - is that right?

Copy link
Contributor

@vlussenburg vlussenburg left a comment

Choose a reason for hiding this comment

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

Although 'boring getter setter logic' this should be unit tested.

Copy link
Contributor

@Dautery91 Dautery91 left a comment

Choose a reason for hiding this comment

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

@konraddysput - looks great overall. The only feedback is to change that tooltip (see my comment). Thanks!

[Tooltip("If the database is unable to send its record, this setting specifies the maximum number of retries before the system gives up")]
public int RetryLimit = 3;

[Tooltip("If the database is unable to send its record, this setting specifies the maximum number of retries before the system gives up")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you copy / pasted the tool tip from the above. Make sure you update it to reflect the real use of the AttachmentPaths field @konraddysput

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah good catch. Thanks

#if UNITY_ANDROID || UNITY_IOS
EditorGUILayout.PropertyField(
serializedObject.FindProperty("AttachmentPaths"),
new GUIContent(BacktraceConfigurationLabels.LABEL_REPORT_ATTACHMENTS));
Copy link
Contributor

Choose a reason for hiding this comment

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

@konraddysput does this end up exposing the array as we discussed in the editor? (The default foldout arrow thing?) If so, this is good for now! Side note - as a part of the work I am doing, I am going to try to come up with some custom property drawers to make these easier to use

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes it does - I hope we can make it prettier soon!

@Dautery91 Dautery91 self-requested a review March 15, 2021 16:27
Copy link
Contributor

@Dautery91 Dautery91 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for making the change!

Base automatically changed from release/3.3.4 to release/3.4.0 March 15, 2021 18:20
@konraddysput konraddysput merged commit 1014e10 into release/3.4.0 Mar 15, 2021
konraddysput added a commit that referenced this pull request Apr 6, 2021
…se-positive anrs (#69)

* Background thread support (#67)

* Background thread support

* Fixed exception flow

* Removed renamed file

* Detect false-positive ANRs report and prevent ANR watchdog from sending them (#66)

* Detect false-positive ANRs report and prevent ANR watchdog from sending them + anr thread improvements

* Fixed typos + better documentation - added Samy's advices

* Attachment support (#68)

* Attachment support on Android - iOS shouldn't compile

* Updated the latest version of backtrace-android libraries and also adjust iOS integration code - code should now compile on iOS

* Native attachment support

* Attachment improvements

* Updated label

* Native client updates

* Android libraries

* Version update

* 3.4.0-rc1

* Prevents from reading empty attachments

* Arabic language support (#70)

* Fixed invalid date in log manager when calendar is unsupported

* Use single method to generate UTC timestamps + move it from extension code to static class to avoid unnecessary allocation

* Fixed line endings

* Updated native library

* Removed debug logs from native library

* Updated library version

* Added invariant culture to rest of the code

* native library update

* Fixed issue in the readme file

* Correct attachment name (#73)

* Correct attachment name

* Moved constants to const variables

* Prepare for final release

* Fixed typo in function name

* Changelog change
@konraddysput konraddysput deleted the feature/attachment-support branch April 7, 2021 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants