Skip to content

Commit

Permalink
feat(deps): sentry@8 (#29)
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
`@sentry/node` peerDependency is updated to 8, which has breaking changes
compared to 7.
  • Loading branch information
kirillgroshkov authored Jan 5, 2025
1 parent 6091300 commit 88ac895
Show file tree
Hide file tree
Showing 7 changed files with 602 additions and 140 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"deploy-health-check-debug2": "yarn tsn ./src/bin/deploy-health-check.ts --url https://api-master2.naturalcycles.com --thresholdUnhealthy 5"
},
"peerDependencies": {
"@sentry/node": "^7"
"@sentry/node": "^8"
},
"dependencies": {
"@naturalcycles/db-lib": "^9",
Expand All @@ -43,7 +43,7 @@
"devDependencies": {
"@naturalcycles/bench-lib": "^3",
"@naturalcycles/dev-lib": "^15",
"@sentry/node": "^7",
"@sentry/node": "^8",
"@types/ejs": "^3",
"@types/node": "^22",
"@types/yargs": "^16",
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/sentry.shared.service.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as sentry from '@sentry/node'
import { SentrySharedService } from './sentry.shared.service'

test('import sentry', async () => {
const _sentryService = new SentrySharedService({})
const _sentryService = new SentrySharedService({ sentry })
})
80 changes: 25 additions & 55 deletions src/sentry/sentry.shared.service.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import {
_anyToError,
_isErrorObject,
_Memo,
CommonLogger,
CommonLogLevel,
Primitive,
StringMap,
} from '@naturalcycles/js-lib'
import { _inspect, InspectAnyOptions } from '@naturalcycles/nodejs-lib'
import type { Breadcrumb, NodeOptions, SeverityLevel } from '@sentry/node'
import type { Breadcrumb, SeverityLevel } from '@sentry/node'
import type * as SentryLib from '@sentry/node'
import { BackendErrorRequestHandler, BackendRequestHandler, getRequestLogger } from '../index'
import { getRequestLogger } from '../index'

export interface SentrySharedServiceCfg extends NodeOptions {}
export interface SentrySharedServiceCfg {
sentry: typeof SentryLib
}

const sentrySeverityMap: Record<SeverityLevel, CommonLogLevel> = {
debug: 'log',
Expand All @@ -28,62 +29,31 @@ const INSPECT_OPT: InspectAnyOptions = {
includeErrorData: true,
}

/**
* Recommended sentry configuration:
*
* {
* maxValueLength: 2000, // default is 250 characters
* }
*
*/
export class SentrySharedService {
constructor(private sentryServiceCfg: SentrySharedServiceCfg) {}

init(): void {
this.sentry()
constructor(sentryServiceCfg: SentrySharedServiceCfg) {
this.sentry = sentryServiceCfg.sentry
}

@_Memo()
sentry(): typeof SentryLib {
// Lazy-loading `@sentry/node`
// Reasons:
// 1. Can be useful is this module is imported but never actually used
// 2. Works around memory leak when used with Jest
const sentry = require('@sentry/node') as typeof SentryLib

if (this.sentryServiceCfg.dsn) {
// Sentry enabled
console.log('SentryService init...')
}

sentry.init({
maxValueLength: 2000, // default is 250 characters
...this.sentryServiceCfg,
})

return sentry
}

/**
* Currently not recommended, because it makes `void` requests throw user-facing errors.
*
* UPD: to be tested. Without it - request is not enriched and the error is less useful.
*/
getRequestHandler(): BackendRequestHandler {
return this.sentry().Handlers.requestHandler()
}

/**
* Currently not recommended, as it's replaced by our custom sentryErrorHandler.
*
* @deprecated
*/
getErrorHandler(): BackendErrorRequestHandler {
return this.sentry().Handlers.errorHandler()
}
sentry: typeof SentryLib

/**
* For GDPR reasons we never send more information than just User ID.
*/
setUserId(id: string | null): void {
if (id === null) {
this.sentry().setUser(null)
this.sentry.setUser(null)
return
}

this.sentry().setUser({
this.sentry.setUser({
id,
})
}
Expand All @@ -98,7 +68,7 @@ export class SentrySharedService {
* https://docs.sentry.io/platforms/node/enriching-events/scopes/
*/
setTags(tags: StringMap<Primitive>): void {
this.sentry().setTags(tags)
this.sentry.setTags(tags)
}

/**
Expand Down Expand Up @@ -131,23 +101,23 @@ export class SentrySharedService {
// This is to avoid Sentry cutting err.message to 253 characters
// It will log additional "breadcrumb object" before the error
// It's a Breadcrumb, not a console.log, because console.log are NOT automatically attached as Breadcrumbs in cron-job environments (outside of Express)
this.sentry().addBreadcrumb({
this.sentry.addBreadcrumb({
message: _inspect(err, INSPECT_OPT),
})

return this.sentry().captureException(err)
return this.sentry.captureException(err)
}

/**
* Returns "eventId"
*/
captureMessage(msg: string, level?: SeverityLevel): string {
getRequestLogger()[sentrySeverityMap[level!] || 'log']('captureMessage:', msg)
return this.sentry().captureMessage(msg, level)
return this.sentry.captureMessage(msg, level)
}

addBreadcrumb(breadcrumb: Breadcrumb): void {
this.sentry().addBreadcrumb(breadcrumb)
this.sentry.addBreadcrumb(breadcrumb)
}

/**
Expand All @@ -164,11 +134,11 @@ export class SentrySharedService {
error: (...args) => {
const message = args.map(arg => _inspect(arg, INSPECT_OPT)).join(' ')

this.sentry().addBreadcrumb({
this.sentry.addBreadcrumb({
message,
})

this.sentry().captureException(_anyToError(args.length === 1 ? args[0] : args))
this.sentry.captureException(_anyToError(args.length === 1 ? args[0] : args))
},
}
}
Expand Down
11 changes: 4 additions & 7 deletions src/server/createDefaultApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,6 @@ export function createDefaultApp(cfg: DefaultAppCfg): BackendApplication {
app.use(asyncLocalStorageMiddleware())
}

// The request handler must be the first middleware on the app
if (sentryService) {
// On error - this handler will set res.headers,
// which will trigger genericErrorHandler "headers already sent"
app.use(sentryService.getRequestHandler())
}

app.use(methodOverrideMiddleware())
app.use(requestTimeoutMiddleware())
// app.use(serverStatsMiddleware()) // disabled by default
Expand Down Expand Up @@ -117,6 +110,10 @@ export function createDefaultApp(cfg: DefaultAppCfg): BackendApplication {
// Generic 404 handler
app.use(notFoundMiddleware())

// todo: test if it's needed!
// Add this after all routes, but before any and other error-handling middlewares are defined
// Sentry.setupExpressErrorHandler(app);

// Generic error handler
// It handles errors, returns proper status, does sentry.captureException(),
// assigns err.data.errorId from sentry
Expand Down
7 changes: 7 additions & 0 deletions src/test/server/instrument.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as sentry from '@sentry/node'

sentry.init({
// no config here
})

export { sentry }
9 changes: 8 additions & 1 deletion src/test/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,15 @@ autocannon -c 100 -d 40 -p 10 localhost:8080
*/

/* eslint-disable simple-import-sort/imports */

console.log('startServer... ')

// should come strictly first
import { sentry } from './instrument'

// should come after `instrument.ts` import

import { _errorLikeToErrorObject, AppError, pDelay } from '@naturalcycles/js-lib'
import { loginHtml } from '../../admin/adminMiddleware'
import {
Expand Down Expand Up @@ -139,7 +146,7 @@ sub.get('/some/:extra', (req, res) => {
})

const sentryService = new SentrySharedService({
autoSessionTracking: false,
sentry,
})

void startServer({
Expand Down
Loading

0 comments on commit 88ac895

Please sign in to comment.