Capture errors in sentry better (#5128)

Fixes #5127 and similar issues. Rationale: Whenever something that is not an Error object is passed to Sentry.captureException we get pretty much no useful information in sentry ![image](https://github.com/compiler-explorer/compiler-explorer/assets/51220084/c7345449-f7a0-46cb-a198-abe0bd80a8b1) (usually not even a stacktrace)
compiler-explorer · Jun 11, 2023 · acdd5ad · acdd5ad
1 parent 319dee6
commit acdd5ad
Show file tree

Hide file tree

Showing 14 changed files with 178 additions and 59 deletions.
diff --git a/app.ts b/app.ts
@@ -62,6 +62,7 @@ import {logger, logToLoki, logToPapertrail, makeLogStream, suppressConsoleLog} f
 import {setupMetricsServer} from './lib/metrics-server.js';
 import {ClientOptionsHandler} from './lib/options-handler.js';
 import * as props from './lib/properties.js';
+import {SetupSentry} from './lib/sentry.js';
 import {ShortLinkResolver} from './lib/shortener/google.js';
 import {sources} from './lib/sources/index.js';
 import {loadSponsorsFromString} from './lib/sponsors.js';
@@ -364,15 +365,6 @@ async function setupStaticMiddleware(router: express.Router) {
     };
 }
 
-function shouldRedactRequestData(data: string) {
-    try {
-        const parsed = JSON.parse(data);
-        return !parsed['allowStoreCodeDebug'];
-    } catch (e) {
-        return true;
-    }
-}
-
 const googleShortUrlResolver = new ShortLinkResolver();
 
 function oldGoogleUrlHandler(req: express.Request, res: express.Response, next: express.NextFunction) {
@@ -451,26 +443,6 @@ function startListening(server: express.Express) {
     }
 }
 
-function setupSentry(sentryDsn: string) {
-    if (!sentryDsn) {
-        logger.info('Not configuring sentry');
-        return;
-    }
-    const sentryEnv = ceProps('sentryEnvironment');
-    Sentry.init({
-        dsn: sentryDsn,
-        release: releaseBuildNumber || gitReleaseName,
-        environment: sentryEnv || defArgs.env[0],
-        beforeSend(event) {
-            if (event.request && event.request.data && shouldRedactRequestData(event.request.data)) {
-                event.request.data = JSON.stringify({redacted: true});
-            }
-            return event;
-        },
-    });
-    logger.info(`Configured with Sentry endpoint ${sentryDsn}`);
-}
-
 const awsProps = props.propsFor('aws');
 
 // eslint-disable-next-line max-statements
@@ -479,7 +451,7 @@ async function main() {
     // Initialise express and then sentry. Sentry as early as possible to catch errors during startup.
     const webServer = express(),
         router = express.Router();
-    setupSentry(aws.getConfig('sentryDsn'));
+    SetupSentry(aws.getConfig('sentryDsn'), ceProps, releaseBuildNumber, gitReleaseName, defArgs);
 
     startWineInit();
 

diff --git a/lib/cache/s3.ts b/lib/cache/s3.ts
@@ -22,12 +22,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-import * as Sentry from '@sentry/node';
-
 import type {GetResult} from '../../types/cache.interfaces.js';
 import {logger} from '../logger.js';
 import {S3Bucket} from '../s3-handler.js';
 import type {S3HandlerOptions} from '../s3-handler.interfaces.js';
+import {SentryCapture} from '../sentry.js';
 
 import {BaseCache} from './base.js';
 import {StorageClass} from '@aws-sdk/client-s3';
@@ -50,7 +49,7 @@ export class S3Cache extends BaseCache {
         this.onError =
             onError ||
             ((e, op) => {
-                Sentry.captureException(e);
+                SentryCapture(e, 'S3Cache onError');
                 logger.error(`Error while trying to ${op} S3 cache: ${messageFor(e)}`);
             });
     }

diff --git a/lib/handlers/api.ts b/lib/handlers/api.ts
@@ -43,6 +43,7 @@ import {withAssemblyDocumentationProviders} from './assembly-documentation.js';
 import {CompileHandler} from './compile.js';
 import {FormattingHandler} from './formatting.js';
 import {getSiteTemplates} from './site-templates.js';
+import {SentryCapture} from '../sentry.js';
 
 function methodNotAllowed(req: express.Request, res: express.Response) {
     res.send('Method Not Allowed');
@@ -166,7 +167,7 @@ export class ApiHandler {
             .catch(err => {
                 logger.warn(`Exception thrown when expanding ${id}: `, err);
                 logger.warn('Exception value:', err);
-                Sentry.captureException(err);
+                SentryCapture(err, 'shortlinkInfoHandler');
                 next({
                     statusCode: 404,
                     message: `ID "${id}" could not be found`,

diff --git a/lib/handlers/compile.ts b/lib/handlers/compile.ts
@@ -50,6 +50,7 @@ import {
 } from './compile.interfaces.js';
 import {remove} from '../common-utils.js';
 import {CompilerOverrideOptions} from '../../types/compilation/compiler-overrides.interfaces.js';
+import {SentryCapture} from '../sentry.js';
 
 temp.track();
 
@@ -170,8 +171,9 @@ export class CompileHandler {
                     ),
                 });
             } else {
-                Sentry.captureException(
+                SentryCapture(
                     new Error(`Unexpected Content-Type received by /compiler/:compiler/compile: ${contentType}`),
+                    'lib/handlers/compile.ts proxyReq contentType',
                 );
                 proxyReq.write('Unexpected Content-Type');
             }
@@ -183,7 +185,7 @@ export class CompileHandler {
                     proxyReq.write(bodyData);
                 }
             } catch (e: any) {
-                Sentry.captureException(e);
+                SentryCapture(e, 'lib/handlers/compile.ts proxyReq.write');
                 let json = '<json stringify error>';
                 try {
                     json = JSON.stringify(bodyData);
@@ -586,7 +588,7 @@ export class CompileHandler {
                                 }
                             }
                         } catch (ex) {
-                            Sentry.captureException(ex);
+                            SentryCapture(ex, 'lib/handlers/compile.ts res.write');
                             res.write(`Error handling request: ${ex}`);
                         }
                         res.end('\n');
@@ -598,7 +600,7 @@ export class CompileHandler {
                     } else {
                         if (error.stack) {
                             logger.error('Error during compilation 2: ', error);
-                            Sentry.captureException(error);
+                            SentryCapture(error, 'compile failed');
                         } else if (error.code) {
                             logger.error('Error during compilation 3: ', error.code);
                             if (typeof error.stderr === 'string') {

diff --git a/lib/handlers/health-check.ts b/lib/handlers/health-check.ts
@@ -22,12 +22,12 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-import * as Sentry from '@sentry/node';
 import express from 'express';
 import fs from 'fs-extra';
 
 import {CompilationQueue} from '../compilation-queue.js';
 import {logger} from '../logger.js';
+import {SentryCapture} from '../sentry.js';
 
 export class HealthCheckHandler {
     public readonly handle: (req: any, res: any) => Promise<void>;
@@ -57,7 +57,7 @@ export class HealthCheckHandler {
             res.send(content);
         } catch (e) {
             logger.error(`*** HEALTH CHECK FAILURE: while reading file '${this.filePath}' got ${e}`);
-            Sentry.captureException(e);
+            SentryCapture(e, 'Health check');
             res.status(500).end();
         }
     }

diff --git a/lib/handlers/route-api.ts b/lib/handlers/route-api.ts
@@ -34,6 +34,7 @@ import {StorageBase} from '../storage/index.js';
 import * as utils from '../utils.js';
 
 import {ApiHandler} from './api.js';
+import {SentryCapture} from '../sentry.js';
 
 export type HandlerConfig = {
     compileHandler: any;
@@ -110,7 +111,7 @@ export class RouteAPI {
             .catch(err => {
                 logger.debug(`Exception thrown when expanding ${id}: `, err);
                 logger.warn('Exception value:', err);
-                Sentry.captureException(err);
+                SentryCapture(err, 'storedCodeHandler');
                 next({
                     statusCode: 404,
                     message: `ID "${id}/${sessionid}" could not be found`,
@@ -201,7 +202,7 @@ export class RouteAPI {
             .catch(err => {
                 logger.warn(`Exception thrown when expanding ${id}`);
                 logger.warn('Exception value:', err);
-                Sentry.captureException(err);
+                SentryCapture(err, 'storedStateHandlerResetLayout');
                 next({
                     statusCode: 404,
                     message: `ID "${id}" could not be found`,

diff --git a/lib/sentry.ts b/lib/sentry.ts
@@ -0,0 +1,85 @@
+// Copyright (c) 2023, Compiler Explorer Authors
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+//     * Redistributions of source code must retain the above copyright notice,
+//       this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+
+import {logger} from './logger.js';
+import {PropertyGetter} from './properties.interfaces.js';
+import {parse} from './stacktrace.js';
+
+import * as Sentry from '@sentry/node';
+
+function shouldRedactRequestData(data: string) {
+    try {
+        const parsed = JSON.parse(data);
+        return !parsed['allowStoreCodeDebug'];
+    } catch (e) {
+        return true;
+    }
+}
+
+export function SetupSentry(
+    sentryDsn: string,
+    ceProps: PropertyGetter,
+    releaseBuildNumber: string | undefined,
+    gitReleaseName: string | undefined,
+    defArgs: any,
+) {
+    if (!sentryDsn) {
+        logger.info('Not configuring sentry');
+        return;
+    }
+    const sentryEnv = ceProps('sentryEnvironment');
+    Sentry.init({
+        dsn: sentryDsn,
+        release: releaseBuildNumber || gitReleaseName,
+        environment: sentryEnv || defArgs.env[0],
+        beforeSend(event) {
+            if (event.request && event.request.data && shouldRedactRequestData(event.request.data)) {
+                event.request.data = JSON.stringify({redacted: true});
+            }
+            return event;
+        },
+    });
+    logger.info(`Configured with Sentry endpoint ${sentryDsn}`);
+}
+
+export function SentryCapture(value: unknown, context?: string) {
+    if (value instanceof Error) {
+        if (context) {
+            value.message += `\nSentryCapture Context: ${context}`;
+        }
+        Sentry.captureException(value);
+    } else {
+        const e = new Error(); // eslint-disable-line unicorn/error-message
+        const trace = parse(e);
+        Sentry.captureMessage(
+            `Non-Error capture:\n` +
+                (context ? `Context: ${context}\n` : '') +
+                `Data:\n${JSON.stringify(value)}\n` +
+                `Trace:\n` +
+                trace
+                    .map(frame => `${frame.functionName} ${frame.fileName}:${frame.lineNumber}:${frame.columnNumber}`)
+                    .join('\n'),
+        );
+    }
+}
diff --git a/static/analytics.ts b/static/analytics.ts
@@ -23,15 +23,9 @@
 // POSSIBILITY OF SUCH DAMAGE.
 
 import {options} from './options.js';
-import * as Sentry from '@sentry/browser';
+import {SetupSentry} from './sentry.js';
 
-if (options.statusTrackingEnabled && options.sentryDsn) {
-    Sentry.init({
-        dsn: options.sentryDsn,
-        release: options.release,
-        environment: options.sentryEnvironment,
-    });
-}
+SetupSentry();
 
 class GAProxy {
     private hasBeenEnabled = false;

diff --git a/static/compiler-service.ts b/static/compiler-service.ts
@@ -38,6 +38,7 @@ import {CompilerInfo} from '../types/compiler.interfaces.js';
 import {CompilationResult, FiledataPair} from '../types/compilation/compilation.interfaces.js';
 import {CompilationStatus} from './compiler-service.interfaces.js';
 import {IncludeDownloads, SourceAndFiles} from './download-service.js';
+import {SentryCapture} from './sentry.js';
 
 const ASCII_COLORS_RE = new RegExp(/\x1B\[[\d;]*m(.\[K)?/g);
 
@@ -123,7 +124,7 @@ export class CompilerService {
                 }
             }
         } catch (e) {
-            Sentry.captureException(e);
+            SentryCapture(e, 'processFromLangAndCompiler');
         }
         // TODO: What now? Found no compilers!
         return {

diff --git a/static/event-hub.ts b/static/event-hub.ts
@@ -27,6 +27,7 @@ import GoldenLayout from 'golden-layout';
 
 import {type EventMap} from './event-map.js';
 import {type Hub} from './hub.js';
+import {SentryCapture} from './sentry.js';
 
 export type EventHubCallback<T extends unknown[]> = (...args: T) => void;
 
@@ -83,7 +84,7 @@ export class EventHub {
                 this.layoutEventHub.off(subscription.evt, subscription.fn, subscription.ctx);
             } catch (e) {
                 Sentry.captureMessage(`Can not unsubscribe from ${subscription.evt.toString()}`);
-                Sentry.captureException(e);
+                SentryCapture(e, 'event hub unsubscribe');
             }
         }
         this.subscriptions = [];

diff --git a/static/main.ts b/static/main.ts
@@ -62,6 +62,7 @@ import {CompilerExplorerOptions} from './global.js';
 import {ComponentConfig, EmptyCompilerState, StateWithId, StateWithLanguage} from './components.interfaces.js';
 
 import * as utils from '../lib/common-utils.js';
+import {SentryCapture} from './sentry.js';
 
 const logos = require.context('../views/resources/logos', false, /\.(png|svg)$/);
 
@@ -212,7 +213,7 @@ function setupButtons(options: CompilerExplorerOptions, hub: Hub) {
             $('#ces .ces-icons').html(data);
         })
         .fail(err => {
-            Sentry.captureException(err);
+            SentryCapture(err, '$.get failed');
         });
 
     $('#ces').on('click', () => {
@@ -594,7 +595,7 @@ function start() {
         layout = new GoldenLayout(config, root);
         hub = new Hub(layout, subLangId, defaultLangId);
     } catch (e) {
-        Sentry.captureException(e);
+        SentryCapture(e, 'goldenlayout/hub setup');
 
         if (document.URL.includes('/z/')) {
             document.location = document.URL.replace('/z/', '/resetlayout/');

diff --git a/static/panes/compiler.ts b/static/panes/compiler.ts
@@ -72,6 +72,7 @@ import fileSaver = require('file-saver');
 import {ICompilerShared} from '../compiler-shared.interfaces.js';
 import {CompilerShared} from '../compiler-shared.js';
 import type {ActiveTools, CompilationRequest, CompilationRequestOptions} from './compiler-request.interfaces.js';
+import {SentryCapture} from '../sentry.js';
 import {LLVMIrBackendOptions} from '../compilation/ir.interfaces.js';
 
 const toolIcons = require.context('../../views/resources/logos', false, /\.(png|svg)$/);
@@ -1030,7 +1031,7 @@ export class Compiler extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Co
             };
         } else {
             // In case this ever stops working, we'll be notified
-            Sentry.captureException(new Error('Context menu hack did not return valid original method'));
+            SentryCapture(new Error('Context menu hack did not return valid original method'));
         }
 
         this.editor.addAction({