getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
815 stars 330 forks source link

Attach a custom stacktrace when capturing an exception #741

Open prof18 opened 4 years ago

prof18 commented 4 years ago

Platform:

Swift:

sentry-cocoa installed with:

Version of sentry-cocoa: 5.2.2


I have the following issue:

I'm writing a sample that uses Sentry for the CrashKiOs library. The library basically lets you create a custom NSException with a custom stacktrace (here for reference) and I want to log this exception with Sentry. But I noticed that this custom stacktrace is never uploaded.

After digging in the source code, I've noticed that the capture method does not use the callStackReturnAddresses of the exception passed in the capture method but it adds it builds it for the current thread.

There is already the possibility to append a custom stacktrace and I haven't found it? If not, there is any possibility to add it in the future? With Firebase Crashalytics or Bugsnag you can but I prefer using Sentry

Steps to reproduce:

To reproduce the issue, I've uploaded the WIP sample. In the same repo you can find also the working sample for Crashalytics and Bugsnag

bruno-garcia commented 4 years ago

Thanks for raising this. We could create the Sentry stacktrace based on the callStackReturnAddresses first, and only as a fallback use our current approach.

philipphofmann commented 4 years ago

Yep we could do that @bruno-garcia. I added this to Asana. We can't give you an ETA though @prof18.

prof18 commented 4 years ago

No, problem. Thanks for the feedback!

philipphofmann commented 3 years ago

It's worth mentioning that a customer uses CrashKiOS for Kotlin Multiplatform.

philipphofmann commented 3 years ago

Make sure to follow this requirement:

As per Sentry policy, the thread that crashed with an exception should not have a stack trace, but instead, the thread_id attribute should be set on the exception and Sentry will connect the two.

See: https://develop.sentry.dev/sdk/event-payloads/threads/

github-actions[bot] commented 3 years ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

philipphofmann commented 2 years ago

Be aware as this change could mess up grouping.

philipphofmann commented 2 years ago

Related to https://github.com/getsentry/sentry-cocoa/issues/1691.

sebmarchand commented 7 months ago

Being able to attach a custom stack trace to events would be quite useful for some type of errors. In my project we're using Chromium's zombie detector to detect some security and stability issues. When a zombie access is detected we get a breadcrumb containing an array of stack address indicating where the dealloc happened. Being able to symbolize this stack trace and add it to the event would be incredibly useful as it'd give us crash reports that clearly indicate where an object has been dealloc'd and where it is being accessed.

I have tried taking a stab at implementing this by adding a new initWithFrames method in SentryStackTrace, but it feels that some important structures used to turn a stack of raw frame pointers into a stack trace are missing from there (e.g. a SentryStacktraceBuilder) and that this is probably an API that should live in the SentrySDK or SentryClient layer?

Having something similar to sentry_event_value_add_stacktrace in sentry-native would be perfect. Any suggestions on how to design this solution?

philipphofmann commented 7 months ago

@sebmarchand, as you mentioned, you can always create your SentryEvent and fill it with whatever stacktrace you like. You can do something like

let event = Event()
event.stacktrace = SentryStacktrace(frames: ..., registers: ...)
SentrySDK.capture(event: event)

You only need to build the event properly so grouping works correctly. You can look at how we create error and exception events in the client. https://github.com/getsentry/sentry-cocoa/blob/d6ff82cd1e1d0d263fb1a7358b09bd0a08df013e/Sources/Sentry/SentryClient.m#L206-L315

One problem could be that we don't attach the debug meta for normal events. Here is the logic in the client https://github.com/getsentry/sentry-cocoa/blob/d6ff82cd1e1d0d263fb1a7358b09bd0a08df013e/Sources/Sentry/SentryClient.m#L652-L657

We could adapt it a bit to add debug meta for captureEvent as well if you need that.

Feel free to ask more questions if it doesn't work properly.

sebmarchand commented 7 months ago

Thanks Philipp! I tried this but the issue is that the SentryStacktrace constructor expects an array of SentryFrame object, and it's not clear to me if we can easily convert a raw stack frame pointer into a SentryFrame ? My current (hacky) approach for this consist in doing something like this in SentryClient.m :

        // frames is a `NSArray<NSValue *> *` 
        uintptr_t *cArray = malloc(sizeof(uintptr_t) * frames.count);
        for (NSUInteger i = 0; i < frames.count; i++) {
            uintptr_t ptr;
            [[frames objectAtIndex:i] getValue:&ptr];
            printf("Pointer: %p\n", ptr);
            cArray[i] = ptr;
        }

        SentryCrashStackCursor stack_cursor;
        sentrycrashsc_initWithBacktrace(&stack_cursor, cArray, (int)frames.count, 0);
        SentryInAppLogic *inAppLogic =
            [[SentryInAppLogic alloc] initWithInAppIncludes:_options.inAppIncludes
                                              inAppExcludes:_options.inAppExcludes];
        SentryCrashStackEntryMapper *crashStackEntryMapper =
            [[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic];
        SentryStacktraceBuilder *stacktraceBuilder =
            [[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper];

        free(cArray);
        event.stacktrace = [stacktraceBuilder retrieveStacktraceFromCursor:stack_cursor];

(this is really just a PoC)

I tried converting my array of raw pointers into an Array of NSFrame by simply initializing the instruction property of these NSFrame but it didn't seemed to be sufficient, hence this more complex approach based on a SentryStacktraceBuilder.

philipphofmann commented 7 months ago

@sebmarchand, hmm, I think now I get it slowly.

Having something similar to sentry_event_value_add_stacktrace in sentry-native would be perfect. Any suggestions on how to design this solution?

So yes, that makes sense. You just have an array of memory addresses and you want to convert that into a SentryFrame, but you also need to find the package of the corresponding memory address. We do something similar in our SentryMetricKitIntegration. Maybe that helps:

https://github.com/getsentry/sentry-cocoa/blob/d6ff82cd1e1d0d263fb1a7358b09bd0a08df013e/Sources/Sentry/SentryMetricKitIntegration.m#L417-L435

Building a helper function similar to what sentry-native has wouldn't be too complicated for us, but maybe for you. Would you be up for a PR, or do you want to leave it to us?

sebmarchand commented 7 months ago

Thanks for the pointers! I don't mind putting up a PR but I'm not super familiar with the Sentry SDK architecture and I could use some guidances on how to design this new helper function (e.g. in which class it should be exposed). Feel free to take a stab at it if you have the bandwidth for it though!

philipphofmann commented 7 months ago

@sebmarchand, I created a new issue for this, see https://github.com/getsentry/sentry-cocoa/issues/3907, but I can't give you an ETA.

sebmarchand commented 7 months ago

Thanks! I'll comment on the other ticket if I end up implementing this myself and/or if I need some pointers to work on a stopgap solution.