Skip to content

Commit

Permalink
fix: Ensure context from previous run is used for native main crashes (
Browse files Browse the repository at this point in the history
  • Loading branch information
timfish committed Aug 2, 2023
1 parent 9eaed5d commit 85f82bc
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 33 deletions.
90 changes: 62 additions & 28 deletions src/main/integrations/sentry-minidump/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { basename, logger, SentryError } from '@sentry/utils';
import { app, crashReporter } from 'electron';

import { mergeEvents } from '../../../common';
import { getEventDefaults } from '../../context';
import { getDefaultEnvironment, getDefaultReleaseName, getEventDefaults } from '../../context';
import { EXIT_REASONS, onChildProcessGone, onRendererProcessGone } from '../../electron-normalize';
import { sentryCachePath } from '../../fs';
import { getRendererProperties, trackRendererProperties } from '../../renderers';
Expand All @@ -14,6 +14,11 @@ import { checkPreviousSession, sessionCrashed } from '../../sessions';
import { BufferedWriteStore } from '../../store';
import { deleteMinidump, getMinidumpLoader, MinidumpLoader } from './minidump-loader';

interface PreviousRun {
scope: Scope;
event?: Event;
}

/** Sends minidumps via the Sentry uploader */
export class SentryMinidump implements Integration {
/** @inheritDoc */
Expand All @@ -23,10 +28,10 @@ export class SentryMinidump implements Integration {
public name: string = SentryMinidump.id;

/** Store to persist context information beyond application crashes. */
private _scopeStore?: BufferedWriteStore<Scope>;
private _scopeStore?: BufferedWriteStore<PreviousRun>;

/** Temp store for the scope of last run */
private _scopeLastRun?: Promise<Scope>;
private _scopeLastRun?: Promise<PreviousRun>;

private _minidumpLoader?: MinidumpLoader;

Expand All @@ -41,15 +46,22 @@ export class SentryMinidump implements Integration {

this._startCrashReporter();

this._scopeStore = new BufferedWriteStore<Scope>(sentryCachePath, 'scope_v2', new Scope());
this._scopeStore = new BufferedWriteStore<PreviousRun>(sentryCachePath, 'scope_v3', {
scope: new Scope(),
});

// We need to store the scope in a variable here so it can be attached to minidumps
this._scopeLastRun = this._scopeStore.get();

this._setupScopeListener();

const client = getCurrentHub().getClient<NodeClient>();
const hub = getCurrentHub();
const client = hub.getClient<NodeClient>();
const options = client?.getOptions() as ElectronMainOptions;

const currentRelease = options?.release || getDefaultReleaseName();
const currentEnvironment = options?.environment || getDefaultEnvironment();

this._setupScopeListener(currentRelease, currentEnvironment);

if (!options?.dsn) {
throw new SentryError('Attempted to enable Electron native crash reporter but no DSN was supplied');
}
Expand Down Expand Up @@ -169,22 +181,31 @@ export class SentryMinidump implements Integration {
/**
* Adds a scope listener to persist changes to disk.
*/
private _setupScopeListener(): void {
const hubScope = getCurrentHub().getScope();
if (hubScope) {
hubScope.addScopeListener((updatedScope) => {
const scope = Scope.clone(updatedScope);
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(scope as any)._eventProcessors = [];
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(scope as any)._scopeListeners = [];

// Since the initial scope read is async, we need to ensure that any writes do not beat that
// https://github.com/getsentry/sentry-electron/issues/585
setImmediate(() => {
void this._scopeStore?.set(scope);
private _setupScopeListener(currentRelease: string, currentEnvironment: string): void {
const scopeChanged = (updatedScope: Scope): void => {
const scope = Scope.clone(updatedScope);
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(scope as any)._eventProcessors = [];
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(scope as any)._scopeListeners = [];

// Since the initial scope read is async, we need to ensure that any writes do not beat that
// https://github.com/getsentry/sentry-electron/issues/585
setImmediate(async () => {
const event = await getEventDefaults(currentRelease, currentEnvironment);
void this._scopeStore?.set({
scope,
event,
});
});
};

const scope = getCurrentHub().getScope();

if (scope) {
scope.addScopeListener(scopeChanged);
// Ensure at least one event is written to disk
scopeChanged(scope);
}
}

Expand All @@ -193,7 +214,7 @@ export class SentryMinidump implements Integration {
*
* Returns true if one or more minidumps were found
*/
private async _sendNativeCrashes(event: Event): Promise<boolean> {
private async _sendNativeCrashes(eventIn: Event): Promise<boolean> {
// Whenever we are called, assume that the crashes we are going to load down
// below have occurred recently. This means, we can use the same event data
// for all minidumps that we load now. There are two conditions:
Expand All @@ -206,6 +227,8 @@ export class SentryMinidump implements Integration {
// about it. Just use the breadcrumbs and context information we have
// right now and hope that the delay was not too long.

let event: Event | null = eventIn;

if (this._minidumpLoader === undefined) {
throw new SentryError('Invariant violation: Native crashes not enabled');
}
Expand All @@ -224,27 +247,38 @@ export class SentryMinidump implements Integration {
const enabled = client.getOptions().enabled;

// If the SDK is not enabled, we delete the minidump files so they
// dont accumulate and/or get sent later
// don't accumulate and/or get sent later
if (enabled === false) {
minidumps.forEach(deleteMinidump);
return false;
}

const storedScope = Scope.clone(await this._scopeLastRun);
let newEvent = await storedScope.applyToEvent(event);
// If this is a native main process crash, we need to apply the scope and context from the previous run
if (event?.tags?.['event.process'] === 'browser') {
const previousRun = await this._scopeLastRun;

const storedScope = Scope.clone(previousRun?.scope);
event = await storedScope.applyToEvent(event);

if (event && previousRun) {
event.release = previousRun.event?.release || event.release;
event.environment = previousRun.event?.environment || event.environment;
event.contexts = previousRun.event?.contexts || event.contexts;
}
}

const hubScope = hub.getScope();
newEvent = hubScope ? await hubScope.applyToEvent(event) : event;
event = hubScope && event ? await hubScope.applyToEvent(event) : event;

if (!newEvent) {
if (!event) {
return false;
}

for (const minidump of minidumps) {
const data = await minidump.load();

if (data) {
captureEvent(newEvent, {
captureEvent(event, {
attachments: [
{
attachmentType: 'event.minidump',
Expand Down
5 changes: 1 addition & 4 deletions src/main/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ export class Store<T> {
* Extends Store to throttle writes.
*/
export class BufferedWriteStore<T> extends Store<T> {
/** The minimum time between writes */
private readonly _throttleTime?: number;
/** A write that hasn't been written to disk yet */
private _pendingWrite: { data: T; timeout: NodeJS.Timeout } | undefined;

Expand All @@ -130,9 +128,8 @@ export class BufferedWriteStore<T> extends Store<T> {
* @param initial An initial value to initialize data with.
* @param throttleTime The minimum time between writes
*/
public constructor(path: string, id: string, initial: T, throttleTime: number = 500) {
public constructor(path: string, id: string, initial: T, private readonly _throttleTime: number = 500) {
super(path, id, initial);
this._throttleTime = throttleTime;
}

/** @inheritdoc */
Expand Down
72 changes: 72 additions & 0 deletions test/e2e/test-apps/native-sentry/main-update/event.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
{
"method": "envelope",
"sentryKey": "37f8a2ee37c0409d8970bc7559c7c7e4",
"appId": "277345",
"data": {
"sdk": {
"name": "sentry.javascript.electron",
"packages": [
{
"name": "npm:@sentry/electron",
"version": "{{version}}"
}
],
"version": "{{version}}"
},
"contexts": {
"app": {
"app_name": "native-sentry-main-update",
"app_version": "1.0.0",
"app_start_time": "{{time}}"
},
"browser": {
"name": "Chrome"
},
"chrome": {
"name": "Chrome",
"type": "runtime",
"version": "{{version}}"
},
"device": {
"arch": "{{arch}}",
"family": "Desktop",
"memory_size": 0,
"free_memory": 0,
"processor_count": 0,
"processor_frequency": 0,
"cpu_description": "{{cpu}}",
"screen_resolution":"{{screen}}",
"screen_density": 1,
"language": "{{language}}"
},
"node": {
"name": "Node",
"type": "runtime",
"version": "{{version}}"
},
"os": {
"name": "{{platform}}",
"version": "{{version}}"
},
"runtime": {
"name": "Electron",
"version": "{{version}}"
}
},
"release": "native-sentry-main-update@1.0.0",
"environment": "development",
"user": {
"ip_address": "{{auto}}"
},
"event_id": "{{id}}",
"timestamp": 0,
"breadcrumbs": [],
"tags": {
"event.environment": "native",
"event.origin": "electron",
"event.process": "browser",
"event_type": "native"
}
},
"attachments": [ { "attachment_type": "event.minidump" } ]
}
8 changes: 8 additions & 0 deletions test/e2e/test-apps/native-sentry/main-update/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "native-sentry-main-update",
"version": "1.0.0",
"main": "src/main.js",
"dependencies": {
"@sentry/electron": "3.0.0"
}
}
4 changes: 4 additions & 0 deletions test/e2e/test-apps/native-sentry/main-update/recipe.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
description: Native Main Crash (after update)
category: Native (Sentry Uploader)
command: yarn
runTwice: true
15 changes: 15 additions & 0 deletions test/e2e/test-apps/native-sentry/main-update/src/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8" />
</head>
<body>
<script>
const { init } = require('@sentry/electron');

init({
debug: true,
});
</script>
</body>
</html>
32 changes: 32 additions & 0 deletions test/e2e/test-apps/native-sentry/main-update/src/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const path = require('path');

const { app, BrowserWindow } = require('electron');
const { init } = require('@sentry/electron');

init({
dsn: '__DSN__',
release: process.env.APP_FIRST_RUN ? 'native-sentry-main-update@1.0.0' : 'native-sentry-main-update@2.0.0',
debug: true,
autoSessionTracking: false,
onFatalError: () => {},
});

app.on('ready', () => {
const mainWindow = new BrowserWindow({
show: false,
webPreferences: {
nodeIntegration: true,
contextIsolation: false,
},
});

mainWindow.loadFile(path.join(__dirname, 'index.html'));

// We only crash on the first run
// The second run is where the crash is uploaded
if (process.env.APP_FIRST_RUN) {
setTimeout(() => {
process.crash();
}, 1000);
}
});
2 changes: 1 addition & 1 deletion test/e2e/test-apps/native-sentry/main/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ app.on('ready', () => {
if (process.env.APP_FIRST_RUN) {
setTimeout(() => {
process.crash();
}, 500);
}, 1000);
}
});

0 comments on commit 85f82bc

Please sign in to comment.