Skip to content

feat: Add synchronous storeCrashReport method to RaygunClient#74

Open
miquelbeltran wants to merge 1 commit intomasterfrom
kmp-fixes
Open

feat: Add synchronous storeCrashReport method to RaygunClient#74
miquelbeltran wants to merge 1 commit intomasterfrom
kmp-fixes

Conversation

@miquelbeltran
Copy link

Purpose

Add a public synchronous storeCrashReport: method to RaygunClient that allows external callers to persist a RaygunMessage to disk. This is needed for capturing Kotlin Multiplatform (KMP) unhandled exceptions on iOS, where the crash report must be written synchronously before the process terminates.

Approach

  • Expose storeCrashReport: as a public method on RaygunClient.
  • Reuse the existing RaygunFileManager storage infrastructure.
  • The method applies beforeSendMessage and groupingKeyProvider callbacks before storing.
  • Returns a BOOL indicating success.

Type of change

  • feat: New feature (non-breaking change which adds functionality)

Updates

  • Added storeCrashReport: method to RaygunClient.h and RaygunClient.m

Test plan

  • Call storeCrashReport: with a manually constructed RaygunMessage and verify the JSON file is written to the crashes directory.
  • On next app launch, verify the stored report is sent to Raygun.

- Expose a public storeCrashReport: method that writes a RaygunMessage
  to disk synchronously
- Applies groupingKeyProvider and beforeSendMessage before storing
- Stored reports are sent on next launch via sendAllStoredCrashReports
- Intended for use in unhandled exception hooks where async sends
  would not complete before process termination

Amp-Thread-ID: https://ampcode.com/threads/T-019c9e51-098e-704c-b1bb-7cd5b9576955
Co-authored-by: Amp <amp@ampcode.com>
return NO;
}

NSString *path = [_fileManager storeCrashReport:message withMaxReportsStored:self.maxReportsStoredOnDevice];
Copy link
Contributor

Choose a reason for hiding this comment

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

This uses direct access to _fileManager whereas the existing sendMessage method (L328, L346) uses self.fileManager property accessor.

Choose a reason for hiding this comment

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

This is probably worthwhile aligning.

}

// Call groupingKeyProvider if it's set
if (self.groupingKeyProvider != nil && message.details != nil) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: sendMessage also checks message != nil in this condition. Not a bug, just noting the divergence.

Choose a reason for hiding this comment

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

Also possibly worthwhile making the same as in sendMessage.

Copy link

@TheRealAgentK TheRealAgentK left a comment

Choose a reason for hiding this comment

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

I think the change is overall fine. Left feedback on @velocitysystems's comments, but I think you could merge this PR either way.

@miquelbeltran
Copy link
Author

As mentioned internally, I will keep the PR open while we test internally and then decide how to move once we collect some crashes

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.

3 participants