Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(deps): sentry@8 #29

Merged
merged 3 commits into from
Jan 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading