Skip to content
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

feat: Add screenshot at crash #1920

Merged
merged 33 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4ba0322
SentryCrash screenshot callback
brustolin Jun 22, 2022
9494db5
add screenshot attachment
brustolin Jun 27, 2022
cc44cca
Merge branch 'master' into feat/crash-screenshot
brustolin Jun 27, 2022
aca1443
Format code
getsentry-bot Jun 27, 2022
a4bad19
Update CHANGELOG.md
brustolin Jun 27, 2022
8c5465c
Merge branch 'feat/crash-screenshot' of https://github.com/getsentry/…
brustolin Jun 27, 2022
66820bc
Apply suggestions from code review
brustolin Jun 28, 2022
23d3536
Improvements with file interactions
brustolin Jun 28, 2022
ae6b9e0
Merge branch 'master' into feat/crash-screenshot
brustolin Jun 28, 2022
d60054c
string format
brustolin Jun 28, 2022
4eadd21
Format code
getsentry-bot Jun 28, 2022
dcfb4f9
Merge branch 'master' into feat/crash-screenshot
brustolin Jun 29, 2022
7f66d8d
Merge branch 'feat/crash-screenshot' of https://github.com/getsentry/…
brustolin Jun 29, 2022
a723c7a
Report sink test
brustolin Jun 29, 2022
636bcbe
integration test
brustolin Jun 30, 2022
511864a
Update SentryScreenshot.m
brustolin Jun 30, 2022
db0865e
Merge branch 'master' into feat/crash-screenshot
brustolin Jun 30, 2022
e2fa1f2
tests
brustolin Jul 1, 2022
ceb5fbe
Format code
getsentry-bot Jul 1, 2022
453dddf
Update SentryCrash.m
brustolin Jul 1, 2022
cbe448f
Merge branch 'feat/crash-screenshot' of https://github.com/getsentry/…
brustolin Jul 1, 2022
60a7c2e
Update SentryCrashReportStore.c
brustolin Jul 1, 2022
5229655
Format code
getsentry-bot Jul 1, 2022
da48b93
Apply suggestions from code review
brustolin Jul 6, 2022
dbf5d0b
testfs
brustolin Jul 6, 2022
bd84076
Merge branch 'feat/crash-screenshot' of https://github.com/getsentry/…
brustolin Jul 6, 2022
64845d0
Update SentryCrash.m
brustolin Jul 6, 2022
6a66e78
Format code
getsentry-bot Jul 6, 2022
0b5531a
Update SentryCrashReportStore_Tests.m
brustolin Jul 6, 2022
9b880f6
Format code
getsentry-bot Jul 6, 2022
8c66ac8
more fixes
brustolin Jul 6, 2022
068179d
Merge branch 'feat/crash-screenshot' of https://github.com/getsentry/…
brustolin Jul 6, 2022
bf6008e
Merge branch 'master' into feat/crash-screenshot
brustolin Jul 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Add screenshot at crash (#1920)

## 7.18.1

### Fixes
Expand Down
1 change: 0 additions & 1 deletion Sources/Sentry/SentryCrashReportConverter.m
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ - (SentryEvent *_Nullable)convertReportToEvent
[NSString stringWithFormat:@"Could not convert report:%@", exception.description];
[SentryLog logWithMessage:errorMessage andLevel:kSentryLevelError];
}

return nil;
}

Expand Down
12 changes: 11 additions & 1 deletion Sources/Sentry/SentryCrashReportSink.m
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#import "SentryCrashReportSink.h"
#import "SentryAttachment.h"
#import "SentryClient.h"
#import "SentryCrash.h"
#import "SentryCrashReportConverter.h"
Expand All @@ -9,6 +10,7 @@
#import "SentryLog.h"
#import "SentrySDK+Private.h"
#import "SentrySDK.h"
#import "SentryScope.h"
#import "SentryThread.h"

@interface
Expand All @@ -33,7 +35,15 @@ - (void)handleConvertedEvent:(SentryEvent *)event
sentReports:(NSMutableArray *)sentReports
{
[sentReports addObject:report];
[SentrySDK captureCrashEvent:event];
SentryScope *scope = [[SentryScope alloc] initWithScope:SentrySDK.currentHub.scope];

if (report[@"screenshots"]) {
brustolin marked this conversation as resolved.
Show resolved Hide resolved
for (NSString *ssPath in report[@"screenshots"]) {
[scope addAttachment:[[SentryAttachment alloc] initWithPath:ssPath]];
Copy link
Member

Choose a reason for hiding this comment

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

h: When do we delete the screenshots? The attachments will be read when the SDK creates the envelope. How do we know that the screenshots are still stored on disk? Maybe it's better to read the attachments already here, and use SentryAttachment .initWithData. Then we could delete them already here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshots are deleted alongside the report. We had a code for that already.
Lets say we fail to transmit the report, on the next attempt the files won't be there, I rather have the current mechanism that deletes it.

}
}

[SentrySDK captureCrashEvent:event withScope:scope];
}

- (void)filterReports:(NSArray *)reports
Expand Down
11 changes: 8 additions & 3 deletions Sources/Sentry/SentryHub.m
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,18 @@ - (nullable SentrySession *)incrementSessionErrors
return sessionCopy;
}

- (void)captureCrashEvent:(SentryEvent *)event
{
[self captureCrashEvent:event withScope:self.scope];
}

/**
* If autoSessionTracking is enabled we want to send the crash and the event together to get proper
* numbers for release health statistics. If there are multiple crash events to be sent on the start
* of the SDK there is currently no way to know which one belongs to the crashed session so we just
* send the session with the first crashed event we receive.
*/
- (void)captureCrashEvent:(SentryEvent *)event
- (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope
{
event.isCrashEvent = YES;

Expand All @@ -226,13 +231,13 @@ - (void)captureCrashEvent:(SentryEvent *)event
// It can be that there is no session yet, because autoSessionTracking was just enabled and
// there is a previous crash on disk. In this case we just send the crash event.
if (nil != crashedSession) {
[client captureCrashEvent:event withSession:crashedSession withScope:self.scope];
[client captureCrashEvent:event withSession:crashedSession withScope:scope];
[fileManager deleteCrashedSession];
return;
}
}

[client captureCrashEvent:event withScope:self.scope];
[client captureCrashEvent:event withScope:scope];
}

- (SentryId *)captureTransaction:(SentryTransaction *)transaction withScope:(SentryScope *)scope
Expand Down
5 changes: 5 additions & 0 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ + (void)captureCrashEvent:(SentryEvent *)event
[SentrySDK.currentHub captureCrashEvent:event];
}

+ (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope
{
[SentrySDK.currentHub captureCrashEvent:event withScope:scope];
}

+ (SentryId *)captureEvent:(SentryEvent *)event
{
return [SentrySDK captureEvent:event withScope:SentrySDK.currentHub.scope];
Expand Down
28 changes: 28 additions & 0 deletions Sources/Sentry/SentryScreenshot.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
#if SENTRY_HAS_UIKIT
# import <UIKit/UIKit.h>

void
saveScreenShots(const char *path)
{
[SentryDependencyContainer.sharedInstance.screenshot
saveScreenShots:[NSString stringWithUTF8String:path]];
}

brustolin marked this conversation as resolved.
Show resolved Hide resolved
@implementation SentryScreenshot

- (NSArray<NSData *> *)appScreenshots
Expand Down Expand Up @@ -38,6 +45,27 @@ @implementation SentryScreenshot
return result;
}

- (void)saveScreenShots:(NSString *)path
{
NSArray<UIWindow *> *windows = [SentryDependencyContainer.sharedInstance.application windows];

for (UIWindow *window in windows) {
brustolin marked this conversation as resolved.
Show resolved Hide resolved
UIGraphicsBeginImageContext(window.frame.size);

int index = 0;
if ([window drawViewHierarchyInRect:window.bounds afterScreenUpdates:false]) {
UIImage *img = UIGraphicsGetImageFromCurrentImageContext();
NSString *name = index == 0
? @"screenshot.png"
: [NSString stringWithFormat:@"screenshot-%i.png", index++ + 1];
NSString *fileName = [path stringByAppendingPathComponent:name];
[UIImagePNGRepresentation(img) writeToFile:fileName atomically:YES];
}

UIGraphicsEndImageContext();
brustolin marked this conversation as resolved.
Show resolved Hide resolved
}
}

@end

#endif
27 changes: 22 additions & 5 deletions Sources/Sentry/SentryScreenshotIntegration.m
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#import "SentryScreenshotIntegration.h"
#import "SentryAttachment.h"
#import "SentryClient+Private.h"
#import "SentryCrashC.h"
#import "SentryDependencyContainer.h"
#import "SentryEvent+Private.h"
#import "SentryEvent.h"
Expand All @@ -10,6 +11,23 @@
#import "SentrySDK+Private.h"

#if SENTRY_HAS_UIKIT

void
saveScreenShot(const char *path)
{
NSString *reportPath = [NSString stringWithUTF8String:path];
if (![NSFileManager.defaultManager fileExistsAtPath:reportPath isDirectory:nil]) {
brustolin marked this conversation as resolved.
Show resolved Hide resolved
brustolin marked this conversation as resolved.
Show resolved Hide resolved
NSError *error = nil;
[NSFileManager.defaultManager createDirectoryAtPath:reportPath
withIntermediateDirectories:YES
attributes:nil
error:&error];
if (error != nil)
return;
}
[SentryDependencyContainer.sharedInstance.screenshot saveScreenShots:reportPath];
}

@implementation SentryScreenshotIntegration

- (void)installWithOptions:(nonnull SentryOptions *)options
Expand All @@ -21,6 +39,8 @@ - (void)installWithOptions:(nonnull SentryOptions *)options

SentryClient *client = [SentrySDK.currentHub getClient];
client.attachmentProcessor = self;

sentrycrash_setSaveScreenShot(&saveScreenShot);
}

- (NSArray<SentryAttachment *> *)processAttachments:(NSArray<SentryAttachment *> *)attachments
Expand All @@ -39,11 +59,8 @@ - (void)installWithOptions:(nonnull SentryOptions *)options
[result addObjectsFromArray:attachments];

for (int i = 0; i < screenshot.count; i++) {
NSString *name;
if (i == 0)
name = @"screenshot.png";
else
name = [NSString stringWithFormat:@"screenshot-%i.png", i + 1];
NSString *name
= i == 0 ? @"screenshot.png" : [NSString stringWithFormat:@"screenshot-%i.png", i + 1];

SentryAttachment *att = [[SentryAttachment alloc] initWithData:screenshot[i]
filename:name
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryHub+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ SentryHub (Private)

- (void)captureCrashEvent:(SentryEvent *)event;

- (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope;

- (void)setSampleRandomValue:(NSNumber *)value;

- (void)closeCachedSessionWithTimestamp:(NSDate *_Nullable)timestamp;
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentrySDK+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ SentrySDK (Private)

+ (void)captureCrashEvent:(SentryEvent *)event;

+ (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope;

/**
* SDK private field to store the state if onCrashedLastRun was called.
*/
Expand Down
1 change: 1 addition & 0 deletions Sources/Sentry/include/SentryScreenshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (nullable NSArray<NSData *> *)appScreenshots;

- (void)saveScreenShots:(NSString *)path;
@end

NS_ASSUME_NONNULL_END
Expand Down
30 changes: 30 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrash.m
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#import "SentryCrashMonitor_AppState.h"
#import "SentryCrashMonitor_System.h"
#import "SentryCrashReportFields.h"
#import "SentryCrashReportStore.h"
#import "SentryCrashSystemCapabilities.h"
#import <NSData+Sentry.h>

Expand Down Expand Up @@ -407,6 +408,28 @@ - (NSData *)loadCrashReportJSONWithID:(int64_t)reportID
return nil;
}

- (NSArray *)getScreenShots:(int64_t)reportID
brustolin marked this conversation as resolved.
Show resolved Hide resolved
{
char report_screenshot_path[SentryCrashCRS_MAX_PATH_LENGTH];
sentrycrashcrs_screenShotPath(reportID, report_screenshot_path);
NSString *path = [NSString stringWithUTF8String:report_screenshot_path];

bool isDir = false;
if (![NSFileManager.defaultManager fileExistsAtPath:path isDirectory:&isDir] || !isDir)
return @[];

NSArray *files = [NSFileManager.defaultManager contentsOfDirectoryAtPath:path error:nil];
if (files == nil)
return @[];

NSMutableArray *result = [[NSMutableArray alloc] initWithCapacity:files.count];
[files enumerateObjectsUsingBlock:^(id _Nonnull obj, NSUInteger idx, BOOL *_Nonnull stop) {
[result addObject:[NSString stringWithFormat:@"%@/%@", path, obj]];
}];

return result;
}

- (void)doctorReport:(NSMutableDictionary *)report
{
NSMutableDictionary *crashReport = report[@SentryCrashField_Crash];
Expand Down Expand Up @@ -452,6 +475,7 @@ - (NSDictionary *)reportWithIntID:(int64_t)reportID
| SentryCrashJSONDecodeOptionIgnoreNullInObject
| SentryCrashJSONDecodeOptionKeepPartialObject
error:&error];

if (error != nil) {
SentryCrashLOG_ERROR(
@"Encountered error loading crash report %" PRIx64 ": %@", reportID, error);
Expand All @@ -460,6 +484,12 @@ - (NSDictionary *)reportWithIntID:(int64_t)reportID
SentryCrashLOG_ERROR(@"Could not load crash report");
return nil;
}

NSArray *screenShots = [self getScreenShots:reportID];
if (screenShots.count > 0) {
[crashReport setObject:screenShots forKey:@"screenshots"];
}

[self doctorReport:crashReport];

return crashReport;
Expand Down
18 changes: 18 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashC.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ static bool g_shouldPrintPreviousLog = false;
static char g_consoleLogPath[SentryCrashFU_MAX_PATH_LENGTH];
static SentryCrashMonitorType g_monitoring = SentryCrashMonitorTypeProductionSafeMinimal;
static char g_lastCrashReportFilePath[SentryCrashFU_MAX_PATH_LENGTH];
static void (*g_saveScreenShot)(const char *) = 0;

// ============================================================================
#pragma mark - Utility -
Expand Down Expand Up @@ -106,6 +107,17 @@ onCrash(struct SentryCrash_MonitorContext *monitorContext)
strncpy(g_lastCrashReportFilePath, crashReportFilePath, sizeof(g_lastCrashReportFilePath));
sentrycrashreport_writeStandardReport(monitorContext, crashReportFilePath);
}

// Report is saved to disk, now we try to take screenshots.
// Depending on the state of the crash this may not work
// because we gonna call into asyng code.
brustolin marked this conversation as resolved.
Show resolved Hide resolved
brustolin marked this conversation as resolved.
Show resolved Hide resolved
if (g_saveScreenShot) {
unsigned long pathLen = strlen(g_lastCrashReportFilePath);
char crashScreenshotsPath[pathLen];
strcpy(crashScreenshotsPath, g_lastCrashReportFilePath);
crashScreenshotsPath[pathLen - 5] = 0; // remove extension to use as a directory
brustolin marked this conversation as resolved.
Show resolved Hide resolved
g_saveScreenShot(crashScreenshotsPath);
}
}

// ============================================================================
Expand Down Expand Up @@ -211,6 +223,12 @@ sentrycrash_setMaxReportCount(int maxReportCount)
sentrycrashcrs_setMaxReportCount(maxReportCount);
}

void
sentrycrash_setSaveScreenShot(void (*callback)(const char *))
{
g_saveScreenShot = callback;
}

void
sentrycrash_reportUserException(const char *name, const char *reason, const char *language,
const char *lineOfCode, const char *stackTrace, bool logAllThreads, bool terminateProgram)
Expand Down
7 changes: 7 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashC.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ void sentrycrash_setPrintPreviousLog(bool shouldPrintPreviousLog);
*/
void sentrycrash_setMaxReportCount(int maxReportCount);

/**
* Set the callback to be called at the end of a crash to make the app save a screenshot;
*
* @param callback function pointer that will be called with a give path.
*/
void sentrycrash_setSaveScreenShot(void (*callback)(const char *));
brustolin marked this conversation as resolved.
Show resolved Hide resolved

/** Report a custom, user defined exception.
* This can be useful when dealing with scripting languages.
*
Expand Down
7 changes: 7 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashReportStore.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ sentrycrashcrs_readReport(int64_t reportID)
return result;
}

void
sentrycrashcrs_screenShotPath(int64_t reportID, char *pathBuffer)
{
snprintf(pathBuffer, SentryCrashCRS_MAX_PATH_LENGTH, "%s/%s-report-%016llx", g_reportsPath,
brustolin marked this conversation as resolved.
Show resolved Hide resolved
g_appName, reportID);
Copy link
Member

Choose a reason for hiding this comment

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

m: Should we maybe also wrap this with pthread_mutex_lock(&g_mutex); and pthread_mutex_unlock(&g_mutex);?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im not sure. The method to get the report file path does not use it

}

int64_t
sentrycrashcrs_addUserReport(const char *report, int reportLength)
{
Expand Down
7 changes: 7 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashReportStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ int sentrycrashcrs_getReportIDs(int64_t *reportIDs, int count);
*/
char *sentrycrashcrs_readReport(int64_t reportID);

/** Gets a report screenshots directory.
*
* @param reportID The report's ID.
* @param pathBuffer A buffer to store the path;
brustolin marked this conversation as resolved.
Show resolved Hide resolved
*/
void sentrycrashcrs_screenShotPath(int64_t reportID, char *pathBuffer);
brustolin marked this conversation as resolved.
Show resolved Hide resolved

/** Add a custom report to the store.
*
* @param report The report's contents (must be JSON encoded).
Expand Down