Switch error tracking to Sentry (#4409)

.bundlewatch.config.json

@@ -2,7 +2,7 @@
     "files": [
         {
             "path": "./dist/assets/owid.mjs",
-            "maxSize": "2.6MB"
+            "maxSize": "2.8MB"
         },
         {
             "path": "./dist/assets/owid.css",

.github/workflows/sentry.yml

@@ -0,0 +1,21 @@
+name: Sentry Release
+on: push
+
+jobs:
+    create-release:
+        runs-on: ubuntu-latest
+
+        steps:
+            - name: Clone repository
+              uses: actions/checkout@v4
+              with:
+                  fetch-depth: 0
+
+            - name: Create Sentry release
+              uses: getsentry/action-release@v1
+              env:
+                  SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
+                  SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
+              with:
+                  environment: ${{ github.ref_name == 'master' && 'production' || 'staging' }}
+                  projects: admin website

.gitignore

@@ -45,3 +45,6 @@ dist/
 **/tsup.config.bundled*.mjs
 cfstorage/
 vite.*.mjs
+
+# Sentry Config File
+.env.sentry-build-plugin

README.md

@@ -84,5 +84,3 @@ The following is an excerpt explaining the origin of this repo and what the alte
 ---
 
 Cross-browser testing provided by <a href="https://www.browserstack.com"><img src="https://3fxtqy18kygf3on3bu39kh93-wpengine.netdna-ssl.com/wp-content/themes/browserstack/img/bs-logo.svg" /> BrowserStack</a>
-
-Client-side bug tracking provided by <a href="http://www.bugsnag.com/"><img width="110" src="https://images.typeform.com/images/QKuaAssrFCq7/image/default" /></a>

adminSiteClient/Admin.tsx

@@ -1,3 +1,4 @@
+import * as Sentry from "@sentry/react"
 import ReactDOM from "react-dom"
 import * as lodash from "lodash"
 import { observable, computed, action } from "mobx"
@@ -6,6 +7,7 @@ import urljoin from "url-join"
 import { AdminApp } from "./AdminApp.js"
 import {
     Json,
+    JsonError,
     stringifyUnknownError,
     queryParamsToStr,
 } from "@ourworldindata/utils"
@@ -30,18 +32,26 @@ export class Admin {
     @observable errorMessage?: ErrorMessage
     basePath: string
     username: string
+    email: string
     isSuperuser: boolean
     settings: ClientSettings
 
     constructor(props: {
         username: string
+        email: string
         isSuperuser: boolean
         settings: ClientSettings
     }) {
         this.basePath = "/admin"
         this.username = props.username
+        this.email = props.email
         this.isSuperuser = props.isSuperuser
         this.settings = props.settings
+
+        Sentry.setUser({
+            username: this.username,
+            email: this.email,
+        })
     }
 
     @observable currentRequests: Promise<Response>[] = []
@@ -131,7 +141,9 @@ export class Admin {
             text = await response.text()
 
             json = JSON.parse(text)
-            if (json.error) throw json.error
+            if (json.error) {
+                throw new JsonError(json.error.message, json.error.status)
+            }
         } catch (err) {
             if (onFailure === "show")
                 this.setErrorMessage({

adminSiteClient/admin.entry.ts

@@ -1,3 +1,7 @@
+// This should be imported as early as possible so the global error handler is
+// set up before any errors are thrown.
+import "./instrument.js"
+
 import "./admin.scss"
 import "@ourworldindata/grapher/src/core/grapher.scss"
 import "../explorerAdminClient/ExplorerCreatePage.scss"

adminSiteClient/instrument.ts

@@ -0,0 +1,12 @@
+import * as Sentry from "@sentry/react"
+import {
+    COMMIT_SHA,
+    ENV,
+    SENTRY_ADMIN_DSN,
+} from "../settings/clientSettings.js"
+
+Sentry.init({
+    dsn: SENTRY_ADMIN_DSN,
+    environment: ENV,
+    release: COMMIT_SHA,
+})

adminSiteServer/IndexPage.tsx

@@ -6,18 +6,20 @@ import {
 import { viteAssetsForAdmin } from "../site/viteUtils.js"
 
 export const IndexPage = (props: {
+    email: string
     username: string
     isSuperuser: boolean
     gitCmsBranchName: string
 }) => {
     const assets = viteAssetsForAdmin()
     const script = `
         window.isEditor = true
-        window.admin = new Admin({ username: "${
-            props.username
-        }", isSuperuser: ${props.isSuperuser.toString()}, settings: ${JSON.stringify(
-            { ENV, GITHUB_USERNAME, DATA_API_FOR_ADMIN_UI }
-        )}})
+        window.admin = new Admin({
+          username: "${props.username}",
+          email: "${props.email}",
+          isSuperuser: ${props.isSuperuser.toString()},
+          settings: ${JSON.stringify({ ENV, GITHUB_USERNAME, DATA_API_FOR_ADMIN_UI })}
+        })
         admin.start(document.querySelector("#app"), '${props.gitCmsBranchName}')
 `
 

adminSiteServer/apiRoutes/datasets.ts

@@ -12,7 +12,7 @@ import {
     assignTagsForCharts,
 } from "../../db/model/Chart.js"
 import { getDatasetById, setTagsForDataset } from "../../db/model/Dataset.js"
-import { logErrorAndMaybeSendToBugsnag } from "../../serverUtils/errorLog.js"
+import { logErrorAndMaybeCaptureInSentry } from "../../serverUtils/errorLog.js"
 import { expectInt } from "../../serverUtils/serverUtil.js"
 import {
     syncDatasetToGitRepo,
@@ -299,7 +299,7 @@ export async function updateDataset(
             commitEmail: _res.locals.user.email,
         })
     } catch (err) {
-        await logErrorAndMaybeSendToBugsnag(err, req)
+        await logErrorAndMaybeCaptureInSentry(err)
         // Continue
     }
 
@@ -363,7 +363,7 @@ export async function deleteDataset(
             commitEmail: _res.locals.user.email,
         })
     } catch (err: any) {
-        await logErrorAndMaybeSendToBugsnag(err, req)
+        await logErrorAndMaybeCaptureInSentry(err)
         // Continue
     }
 

adminSiteServer/app.ts

@@ -1,3 +1,7 @@
+// This should be imported as early as possible so the global error handler is
+// set up before any errors are thrown.
+import "../serverUtils/instrument.js"
+
 import { GIT_CMS_DIR } from "../gitCms/GitCmsConstants.js"
 import {
     ADMIN_SERVER_HOST,

adminSiteServer/appClass.tsx

@@ -1,16 +1,11 @@
 import { simpleGit } from "simple-git"
 import express, { NextFunction } from "express"
+import * as Sentry from "@sentry/node"
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 require("express-async-errors") // todo: why the require?
 import cookieParser from "cookie-parser"
 import http from "http"
-import Bugsnag from "@bugsnag/js"
-import BugsnagPluginExpress from "@bugsnag/plugin-express"
-import {
-    BAKED_BASE_URL,
-    BUGSNAG_NODE_API_KEY,
-    ENV_IS_STAGING,
-} from "../settings/serverSettings.js"
+import { BAKED_BASE_URL, ENV_IS_STAGING } from "../settings/serverSettings.js"
 import * as db from "../db/db.js"
 import { IndexPage } from "./IndexPage.js"
 import {
@@ -66,23 +61,9 @@ export class OwidAdminApp {
 
     async startListening(adminServerPort: number, adminServerHost: string) {
         this.gitCmsBranchName = await this.getGitCmsBranchName()
-        let bugsnagMiddleware
 
         const { app } = this
 
-        if (BUGSNAG_NODE_API_KEY) {
-            Bugsnag.start({
-                apiKey: BUGSNAG_NODE_API_KEY,
-                context: "admin-server",
-                plugins: [BugsnagPluginExpress],
-                autoTrackSessions: false,
-            })
-            bugsnagMiddleware = Bugsnag.getPlugin("express")
-            // From the docs: "this must be the first piece of middleware in the
-            // stack. It can only capture errors in downstream middleware"
-            if (bugsnagMiddleware) app.use(bugsnagMiddleware.requestHandler)
-        }
-
         // since the server is running behind a reverse proxy (nginx), we need to "trust"
         // the X-Forwarded-For header in order to get the real request IP
         // https://expressjs.com/en/guide/behind-proxies.html
@@ -119,6 +100,7 @@ export class OwidAdminApp {
             res.send(
                 renderToHtmlPage(
                     <IndexPage
+                        email={res.locals.user.email}
                         username={res.locals.user.fullName}
                         isSuperuser={res.locals.user.isSuperuser}
                         gitCmsBranchName={this.gitCmsBranchName}
@@ -157,11 +139,6 @@ export class OwidAdminApp {
             }
         })
 
-        // From the docs: "this handles any errors that Express catches. This
-        // needs to go before other error handlers. BugSnag will call the `next`
-        // error handler if it exists.
-        if (bugsnagMiddleware) app.use(bugsnagMiddleware.errorHandler)
-
         if (this.options.isDev) {
             if (!this.options.isTest) {
                 // https://vitejs.dev/guide/ssr
@@ -179,6 +156,10 @@ export class OwidAdminApp {
             app.use("/", mockSiteRouter)
         }
 
+        // Add this after all routes, but before any other error-handling
+        // middlewares are defined.
+        Sentry.setupExpressErrorHandler(app)
+
         // Give full error messages, including in production
         app.use(this.errorHandler)
 

adminSiteServer/authentication.ts

@@ -1,3 +1,4 @@
+import * as Sentry from "@sentry/node"
 import express from "express"
 import crypto from "crypto"
 import randomstring from "randomstring"
@@ -100,7 +101,7 @@ export async function authCloudflareSSOMiddleware(
     res.cookie("sessionid", sessionId, {
         httpOnly: true,
         sameSite: "lax",
-        secure: ENV === "production",
+        secure: ENV !== "development",
     })
 
     // Prevents redirect to external URLs
@@ -194,6 +195,13 @@ export async function authMiddleware(
     if (user?.isActive) {
         res.locals.session = session
         res.locals.user = user
+
+        Sentry.setUser({
+            id: user.id,
+            email: user.email,
+            username: user.fullName,
+        })
+
         return next()
     } else if (!req.path.startsWith("/admin") || req.path === "/admin/login")
         return next()
@@ -327,7 +335,7 @@ export async function tailscaleAuthMiddleware(
     res.cookie("sessionid", sessionId, {
         httpOnly: true,
         sameSite: "lax",
-        secure: ENV === "production",
+        secure: ENV !== "development",
     })
 
     // Save the sessionid in cookies for `authMiddleware` to log us in

adminSiteServer/chartConfigR2Helpers.ts

@@ -15,7 +15,6 @@ import {
 } from "@aws-sdk/client-s3"
 import { JsonError, lazy } from "@ourworldindata/utils"
 import { Base64String, R2GrapherConfigDirectory } from "@ourworldindata/types"
-import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js"
 
 const getS3Client: () => S3Client = lazy(
     () =>
@@ -116,7 +115,6 @@ async function saveConfigToR2(
             `Successfully uploaded object: ${params.Bucket}/${params.Key}`
         )
     } catch (err) {
-        await logErrorAndMaybeSendToBugsnag(err)
         throw new JsonError(
             `Failed to save the config to R2. Inner error: ${err}`
         )
@@ -162,7 +160,6 @@ export async function deleteGrapherConfigFromR2(
             `Successfully deleted object: ${params.Bucket}/${params.Key}`
         )
     } catch (err) {
-        await logErrorAndMaybeSendToBugsnag(err)
         throw new JsonError(
             `Failed to delete the grapher config to R2 at ${directory}/${filename}. Inner error: ${err}`
         )

baker/DatapageHelpers.ts

@@ -25,7 +25,7 @@ import {
 } from "@ourworldindata/types"
 import { KnexReadonlyTransaction } from "../db/db.js"
 import { parseFaqs } from "../db/model/Gdoc/rawToEnriched.js"
-import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js"
+import { logErrorAndMaybeCaptureInSentry } from "../serverUtils/errorLog.js"
 import { getSlugForTopicTag } from "./GrapherBakingUtils.js"
 import { getShortPageCitation } from "../site/gdocs/utils.js"
 
@@ -197,8 +197,10 @@ export const getPrimaryTopic = async (
     try {
         topicSlug = await getSlugForTopicTag(knex, firstTopicTag)
     } catch {
-        await logErrorAndMaybeSendToBugsnag(
-            `Data page is using "${firstTopicTag}" as its primary tag, which we are unable to resolve to a tag in the grapher DB`
+        await logErrorAndMaybeCaptureInSentry(
+            new Error(
+                `Data page is using "${firstTopicTag}" as its primary tag, which we are unable to resolve to a tag in the grapher DB`
+            )
         )
         return undefined
     }

baker/DeployUtils.ts

@@ -1,6 +1,5 @@
 import fs from "fs-extra"
 import { BuildkiteTrigger } from "../baker/BuildkiteTrigger.js"
-import { logErrorAndMaybeSendToBugsnag, warn } from "../serverUtils/errorLog.js"
 import { DeployQueueServer } from "./DeployQueueServer.js"
 import {
     BAKED_SITE_DIR,
@@ -24,7 +23,7 @@ export const defaultCommitMessage = async (): Promise<string> => {
         const sha = await fs.readFile("public/head.txt", "utf8")
         message += `\nowid/owid-grapher@${sha}`
     } catch (err) {
-        warn(err)
+        console.warn(err)
     }
 
     return message
@@ -43,16 +42,12 @@ const triggerBakeAndDeploy = async (
     if (BUILDKITE_API_ACCESS_TOKEN) {
         const buildkite = new BuildkiteTrigger()
         if (lightningQueue?.length) {
-            await buildkite
-                .runLightningBuild(
-                    lightningQueue.map((change) => change.slug!),
-                    deployMetadata
-                )
-                .catch(logErrorAndMaybeSendToBugsnag)
+            await buildkite.runLightningBuild(
+                lightningQueue.map((change) => change.slug!),
+                deployMetadata
+            )
         } else {
-            await buildkite
-                .runFullBuild(deployMetadata)
-                .catch(logErrorAndMaybeSendToBugsnag)
+            await buildkite.runFullBuild(deployMetadata)
         }
     } else {
         // otherwise, bake locally. This is used for local development or staging servers
@@ -143,11 +138,8 @@ const getSlackMentionByEmail = async (
         const response = await slackClient.users.lookupByEmail({ email })
         return response.user?.id ? `<@${response.user.id}>` : undefined
     } catch (error) {
-        await logErrorAndMaybeSendToBugsnag(
-            `Error looking up email "${email}" in slack: ${error}`
-        )
+        throw new Error(`Error looking up email "${email}" in slack: ${error}`)
     }
-    return
 }
 
 const MAX_SUCCESSIVE_FAILURES = 2