Skip to content

Commit

Permalink
Switch error tracking to Sentry
Browse files Browse the repository at this point in the history
  • Loading branch information
rakyi committed Jan 13, 2025
1 parent 61b3820 commit d67fc63
Show file tree
Hide file tree
Showing 46 changed files with 1,571 additions and 678 deletions.
2 changes: 1 addition & 1 deletion .bundlewatch.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"files": [
{
"path": "./dist/assets/owid.mjs",
"maxSize": "2.6MB"
"maxSize": "2.8MB"
},
{
"path": "./dist/assets/owid.css",
Expand Down
21 changes: 21 additions & 0 deletions .github/workflows/sentry.yml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ dist/
**/tsup.config.bundled*.mjs
cfstorage/
vite.*.mjs

# Sentry Config File
.env.sentry-build-plugin
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
4 changes: 4 additions & 0 deletions adminSiteClient/admin.entry.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
7 changes: 7 additions & 0 deletions adminSiteClient/instrument.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Sentry from "@sentry/react"

Sentry.init({
dsn: process.env.SENTRY_ADMIN_DSN,
environment: process.env.ENV,
release: process.env.COMMIT_SHA,
})
6 changes: 3 additions & 3 deletions adminSiteServer/apiRoutes/datasets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -299,7 +299,7 @@ export async function updateDataset(
commitEmail: _res.locals.user.email,
})
} catch (err) {
await logErrorAndMaybeSendToBugsnag(err, req)
await logErrorAndMaybeCaptureInSentry(err)
// Continue
}

Expand Down Expand Up @@ -363,7 +363,7 @@ export async function deleteDataset(
commitEmail: _res.locals.user.email,
})
} catch (err: any) {
await logErrorAndMaybeSendToBugsnag(err, req)
await logErrorAndMaybeCaptureInSentry(err)
// Continue
}

Expand Down
3 changes: 3 additions & 0 deletions adminSiteServer/app.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// This must be imported first for Sentry instrumentation to work.
import "../serverUtils/instrument.js"

import { GIT_CMS_DIR } from "../gitCms/GitCmsConstants.js"
import {
ADMIN_SERVER_HOST,
Expand Down
32 changes: 6 additions & 26 deletions adminSiteServer/appClass.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -157,11 +138,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
Expand All @@ -179,6 +155,10 @@ export class OwidAdminApp {
app.use("/", mockSiteRouter)
}

// Add this after all routes,
// but before any and other error-handling middlewares are defined
Sentry.setupExpressErrorHandler(app)

// Give full error messages, including in production
app.use(this.errorHandler)

Expand Down
4 changes: 2 additions & 2 deletions adminSiteServer/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export async function authCloudflareSSOMiddleware(
res.cookie("sessionid", sessionId, {
httpOnly: true,
sameSite: "lax",
secure: ENV === "production",
secure: ENV !== "development",
})

// Prevents redirect to external URLs
Expand Down Expand Up @@ -327,7 +327,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
Expand Down
3 changes: 0 additions & 3 deletions adminSiteServer/chartConfigR2Helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
() =>
Expand Down Expand Up @@ -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}`
)
Expand Down Expand Up @@ -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}`
)
Expand Down
8 changes: 5 additions & 3 deletions baker/DatapageHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
}
Expand Down
22 changes: 7 additions & 15 deletions baker/DeployUtils.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions baker/GrapherBaker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
getRelatedResearchAndWritingForVariable,
} from "../db/model/Post.js"
import {
JsonError,
GrapherInterface,
OwidVariableDataMetadataDimensions,
DimensionProperty,
Expand All @@ -54,7 +53,7 @@ import {
resolveFaqsForVariable,
} from "./DatapageHelpers.js"
import { getAllImages } from "../db/model/Image.js"
import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js"
import { logErrorAndMaybeCaptureInSentry } from "../serverUtils/errorLog.js"

import { getTagToSlugMap } from "./GrapherBakingUtils.js"
import { knexRaw } from "../db/db.js"
Expand Down Expand Up @@ -156,8 +155,8 @@ export async function renderDataPageV2(

if (faqResolveErrors.length > 0) {
for (const error of faqResolveErrors) {
await logErrorAndMaybeSendToBugsnag(
new JsonError(
await logErrorAndMaybeCaptureInSentry(
new Error(
`Data page error in finding FAQs for variable ${variableId}: ${error.error}`
)
)
Expand Down
9 changes: 4 additions & 5 deletions baker/GrapherBakingUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { BAKED_SITE_EXPORTS_BASE_URL } from "../settings/clientSettings.js"

import * as db from "../db/db.js"
import { bakeGraphersToSvgs } from "../baker/GrapherImageBaker.js"
import { warn } from "../serverUtils/errorLog.js"
import { mapSlugsToIds } from "../db/model/Chart.js"
import md5 from "md5"
import { DbPlainTag, Url } from "@ourworldindata/utils"
Expand Down Expand Up @@ -70,13 +69,13 @@ export const bakeGrapherUrls = async (

const slug = lodash.last(Url.fromURL(url).pathname?.split("/"))
if (!slug) {
warn(`Invalid chart url ${url}`)
console.warn(`Invalid chart url ${url}`)
continue
}

const chartId = slugToId[slug]
if (chartId === undefined) {
warn(`Couldn't find chart with slug ${slug}`)
console.warn(`Couldn't find chart with slug ${slug}`)
continue
}

Expand All @@ -91,7 +90,7 @@ export const bakeGrapherUrls = async (
[chartId]
)
if (!rows.length) {
warn(`Mysteriously missing chart by id ${chartId}`)
console.warn(`Mysteriously missing chart by id ${chartId}`)
continue
}

Expand Down Expand Up @@ -167,7 +166,7 @@ export async function getTagToSlugMap(

/**
* Given a topic tag's name or ID, return its slug
* Throws an error if no slug is found so we can log it in Bugsnag
* Throws an error if no slug is found so we can log it in Sentry
*/
export async function getSlugForTopicTag(
knex: db.KnexReadonlyTransaction,
Expand Down
7 changes: 3 additions & 4 deletions baker/MultiDimBaker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
} from "@ourworldindata/types"
import {
MultiDimDataPageConfig,
JsonError,
keyBy,
OwidVariableWithSource,
pick,
Expand All @@ -28,7 +27,7 @@ import {
getPrimaryTopic,
resolveFaqsForVariable,
} from "./DatapageHelpers.js"
import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js"
import { logErrorAndMaybeCaptureInSentry } from "../serverUtils/errorLog.js"
import {
getAllMultiDimDataPages,
getMultiDimDataPageBySlug,
Expand Down Expand Up @@ -83,8 +82,8 @@ const getFaqEntries = async (

if (faqResolveErrors.length > 0) {
for (const error of faqResolveErrors) {
void logErrorAndMaybeSendToBugsnag(
new JsonError(
void logErrorAndMaybeCaptureInSentry(
new Error(
`MDD baking error for page "${config.title}" in finding FAQs for variable ${metadata.id}: ${error.error}`
)
)
Expand Down
Loading

0 comments on commit d67fc63

Please sign in to comment.