-
-
Notifications
You must be signed in to change notification settings - Fork 0
Reproduction for sentry-javascript#17742 #23
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
Open
nicohrubec
wants to merge
5
commits into
main
Choose a base branch
from
repro/sentry-javascript-17742
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2f5cf1d
Add reproduction for sentry-javascript#17742
nicohrubec 275cfa6
Align reproduction with official Sentry NestJS docs
nicohrubec 717f074
Reproduce background job breadcrumb leaking into HTTP requests
nicohrubec 3272650
Extend reproduction to cover all NestJS background job frameworks
nicohrubec 7f1f96b
Fix graphile worker runner not starting and increase wait time
nicohrubec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Reproduction for sentry-javascript#17742 | ||
|
|
||
| **Issue:** https://github.com/getsentry/sentry-javascript/issues/17742 | ||
|
|
||
| ## Description | ||
|
|
||
| Breadcrumbs from background jobs leak into HTTP request error events in NestJS. Background jobs run outside the HTTP request context, so they add breadcrumbs to the default isolation scope. When a new HTTP request arrives, `httpServerIntegration` clones the default scope — inheriting all those stale breadcrumbs. | ||
|
|
||
| This reproduction covers **all four** common NestJS background job patterns: | ||
|
|
||
| | Framework | Decorator | External Dep | Env Var | | ||
| |-----------|-----------|-------------|---------| | ||
| | `@nestjs/schedule` | `@Interval` / `@Cron` | None | Always active | | ||
| | `@nestjs/event-emitter` | `@OnEvent` | None | Always active | | ||
| | `@nestjs/bullmq` | `@Processor` | Redis | `REDIS_URL` | | ||
| | `nestjs-graphile-worker` | `@Task` | PostgreSQL | `DATABASE_URL` | | ||
|
|
||
| ## Steps to Reproduce | ||
|
|
||
| 1. Add a `.env` file with your Sentry DSN (and optionally Redis/PostgreSQL): | ||
| ```bash | ||
| SENTRY_DSN=<your-dsn> | ||
| # Optional: | ||
| # REDIS_URL=redis://localhost:6379 | ||
| # DATABASE_URL=postgres://user:pass@localhost:5432/dbname | ||
| ``` | ||
|
|
||
| 2. Install dependencies and run: | ||
| ```bash | ||
| npm install | ||
| npm run test:repro | ||
| ``` | ||
|
|
||
| 3. Check the output for leaked breadcrumbs. | ||
|
|
||
| ## Expected Behavior | ||
|
|
||
| The error event from `GET /trigger-error` should only contain its own breadcrumb: | ||
| ``` | ||
| === Sentry Event Breadcrumbs (1 total) === | ||
| [0] category=http-request, message=About to trigger an error in HTTP handler | ||
| ``` | ||
|
|
||
| ## Actual Behavior | ||
|
|
||
| ``` | ||
| *** BUG CONFIRMED: Breadcrumbs leaked from background jobs! *** | ||
| Leaked: schedule-job: 3, event-job: 2 | ||
| ``` | ||
|
|
||
| ## Root Cause | ||
|
|
||
| In `packages/node-core/src/integrations/http/httpServerIntegration.ts:185`: | ||
| ```ts | ||
| const isolationScope = getIsolationScope().clone(); | ||
| ``` | ||
|
|
||
| Background jobs execute on the default isolation scope (no HTTP request forked a new one). Their breadcrumbs accumulate on the default scope. When `httpServerIntegration` handles a new request, it clones the default scope — including all stale breadcrumbs from background jobs. | ||
|
|
||
| ## Environment | ||
|
|
||
| - Node.js: v18+ | ||
| - @sentry/nestjs: ^10.2.0 | ||
| - @nestjs/core: ^10.0.0 | ||
| - @nestjs/schedule: ^6.1.1 | ||
| - @nestjs/event-emitter: latest | ||
| - @nestjs/bullmq: latest (optional) | ||
| - nestjs-graphile-worker: latest (optional) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "$schema": "https://json.schemastore.org/nest-cli", | ||
| "collection": "@nestjs/schematics", | ||
| "sourceRoot": "src" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| { | ||
| "name": "repro-sentry-javascript-17742", | ||
| "version": "1.0.0", | ||
| "description": "Reproduction for sentry-javascript#17742 - NestJS leaking breadcrumbs", | ||
| "private": true, | ||
| "scripts": { | ||
| "build": "nest build", | ||
| "start": "nest start", | ||
| "test:repro": "npm run build && bash test-repro.sh" | ||
| }, | ||
| "dependencies": { | ||
| "@nestjs/bullmq": "^11.0.4", | ||
| "@nestjs/common": "^10.0.0", | ||
| "@nestjs/core": "^10.0.0", | ||
| "@nestjs/event-emitter": "^3.0.1", | ||
| "@nestjs/platform-express": "^10.0.0", | ||
| "@nestjs/schedule": "^6.1.1", | ||
| "@sentry/nestjs": "^10.2.0", | ||
| "bullmq": "^5.69.3", | ||
| "dotenv": "^17.3.1", | ||
| "graphile-worker": "^0.16.6", | ||
| "nestjs-graphile-worker": "^0.9.1", | ||
| "reflect-metadata": "^0.2.2", | ||
| "rxjs": "^7.8.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@nestjs/cli": "^10.0.0", | ||
| "typescript": "^5.0.0" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { Controller, Get } from "@nestjs/common"; | ||
| import { AppService } from "./app.service"; | ||
|
|
||
| @Controller() | ||
| export class AppController { | ||
| constructor(private readonly appService: AppService) {} | ||
|
|
||
| @Get("trigger-error") | ||
| triggerError(): string { | ||
| return this.appService.triggerError(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import { DynamicModule, Module } from "@nestjs/common"; | ||
| import { APP_FILTER } from "@nestjs/core"; | ||
| import { EventEmitterModule } from "@nestjs/event-emitter"; | ||
| import { ScheduleModule } from "@nestjs/schedule"; | ||
| import { SentryGlobalFilter, SentryModule } from "@sentry/nestjs/setup"; | ||
| import { AppController } from "./app.controller"; | ||
| import { AppService } from "./app.service"; | ||
| import { ScheduleJobService } from "./schedule-job.service"; | ||
| import { EventJobService } from "./event-job.service"; | ||
|
|
||
| /** | ||
| * Build imports and providers dynamically based on available services. | ||
| * - Schedule + EventEmitter: always enabled (no external deps) | ||
| * - BullMQ: enabled when REDIS_URL is set | ||
| * - Graphile Worker: enabled when DATABASE_URL is set | ||
| */ | ||
| function getOptionalImports(): DynamicModule[] { | ||
| const imports: DynamicModule[] = []; | ||
|
|
||
| if (process.env.REDIS_URL) { | ||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { BullModule } = require("@nestjs/bullmq"); | ||
| imports.push( | ||
| BullModule.forRoot({ connection: { url: process.env.REDIS_URL } }) | ||
| ); | ||
| imports.push(BullModule.registerQueue({ name: "background-queue" })); | ||
| console.log("[Config] BullMQ enabled (REDIS_URL set)"); | ||
| } catch (e) { | ||
| console.log("[Config] BullMQ not available"); | ||
| } | ||
| } else { | ||
| console.log("[Config] BullMQ disabled (set REDIS_URL to enable)"); | ||
| } | ||
|
|
||
| if (process.env.DATABASE_URL) { | ||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { GraphileWorkerModule } = require("nestjs-graphile-worker"); | ||
| imports.push( | ||
| GraphileWorkerModule.forRoot({ | ||
| connectionString: process.env.DATABASE_URL, | ||
| }) | ||
| ); | ||
| console.log("[Config] Graphile Worker enabled (DATABASE_URL set)"); | ||
| } catch (e) { | ||
| console.log("[Config] Graphile Worker not available"); | ||
| } | ||
| } else { | ||
| console.log("[Config] Graphile Worker disabled (set DATABASE_URL to enable)"); | ||
| } | ||
|
|
||
| return imports; | ||
| } | ||
|
|
||
| function getOptionalProviders(): any[] { | ||
| const providers: any[] = []; | ||
|
|
||
| if (process.env.REDIS_URL) { | ||
| try { | ||
| const { BullmqJobProcessor, BullmqJobProducer } = require("./bullmq-job.processor"); | ||
| providers.push(BullmqJobProcessor, BullmqJobProducer); | ||
| } catch (e) {} | ||
| } | ||
|
|
||
| if (process.env.DATABASE_URL) { | ||
| try { | ||
| const { GraphileJobHandler, GraphileJobProducer } = require("./graphile-job.service"); | ||
| providers.push(GraphileJobHandler, GraphileJobProducer); | ||
| } catch (e) {} | ||
| } | ||
|
|
||
| return providers; | ||
| } | ||
|
|
||
| @Module({ | ||
| imports: [ | ||
| SentryModule.forRoot(), | ||
| ScheduleModule.forRoot(), | ||
| EventEmitterModule.forRoot(), | ||
| ...getOptionalImports(), | ||
| ], | ||
| controllers: [AppController], | ||
| providers: [ | ||
| { | ||
| provide: APP_FILTER, | ||
| useClass: SentryGlobalFilter, | ||
| }, | ||
| AppService, | ||
| ScheduleJobService, | ||
| EventJobService, | ||
| ...getOptionalProviders(), | ||
| ], | ||
| }) | ||
| export class AppModule {} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { Injectable } from "@nestjs/common"; | ||
| import * as Sentry from "@sentry/nestjs"; | ||
|
|
||
| @Injectable() | ||
| export class AppService { | ||
| triggerError(): string { | ||
| // Add a breadcrumb specific to this HTTP request | ||
| Sentry.addBreadcrumb({ | ||
| category: "http-request", | ||
| message: "About to trigger an error in HTTP handler", | ||
| level: "error", | ||
| }); | ||
|
|
||
| // Throw an exception — the SentryGlobalFilter captures it automatically. | ||
| // EXPECTED: Only the "http-request" breadcrumb should appear on the event | ||
| // ACTUAL (BUG): Breadcrumbs from background jobs leak into this event | ||
| // because the HTTP request's isolation scope was cloned from the default | ||
| // scope, which was polluted by background job breadcrumbs | ||
| throw new Error("Test error to check breadcrumb isolation"); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { Processor, WorkerHost } from "@nestjs/bullmq"; | ||
| import { Injectable, OnModuleInit } from "@nestjs/common"; | ||
| import { InjectQueue } from "@nestjs/bullmq"; | ||
| import { Queue, Job } from "bullmq"; | ||
| import * as Sentry from "@sentry/nestjs"; | ||
|
|
||
| /** | ||
| * Case 3: @nestjs/bullmq | ||
| * BullMQ processors run outside HTTP request context on the default isolation scope. | ||
| */ | ||
| @Processor("background-queue") | ||
| export class BullmqJobProcessor extends WorkerHost { | ||
| private jobCount = 0; | ||
|
|
||
| async process(job: Job): Promise<void> { | ||
| this.jobCount++; | ||
| console.log( | ||
| `[@nestjs/bullmq] Processing job #${this.jobCount}: ${job.name}` | ||
| ); | ||
|
|
||
| Sentry.addBreadcrumb({ | ||
| category: "bullmq-job", | ||
| message: `BullMQ job #${this.jobCount} processed`, | ||
| level: "info", | ||
| }); | ||
|
|
||
| console.log(`[@nestjs/bullmq] Job #${this.jobCount} done`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Service that periodically adds jobs to the BullMQ queue. | ||
| */ | ||
| @Injectable() | ||
| export class BullmqJobProducer implements OnModuleInit { | ||
| private jobCount = 0; | ||
|
|
||
| constructor(@InjectQueue("background-queue") private queue: Queue) {} | ||
|
|
||
| onModuleInit() { | ||
| setInterval(async () => { | ||
| this.jobCount++; | ||
| await this.queue.add("background-task", { | ||
| id: this.jobCount, | ||
| }); | ||
| console.log(`[@nestjs/bullmq] Added job #${this.jobCount} to queue`); | ||
| }, 5000); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { Injectable } from "@nestjs/common"; | ||
| import { EventEmitter2 } from "@nestjs/event-emitter"; | ||
| import { OnEvent } from "@nestjs/event-emitter"; | ||
| import * as Sentry from "@sentry/nestjs"; | ||
|
|
||
| /** | ||
| * Case 2: @nestjs/event-emitter | ||
| * Event handlers run outside HTTP request context on the default isolation scope. | ||
| */ | ||
| @Injectable() | ||
| export class EventJobService { | ||
| private eventCount = 0; | ||
|
|
||
| constructor(private eventEmitter: EventEmitter2) { | ||
| // Emit events periodically to simulate background event processing | ||
| setInterval(() => { | ||
| this.eventEmitter.emit("background.task", { | ||
| id: Date.now(), | ||
| }); | ||
| }, 4000); | ||
| } | ||
|
|
||
| @OnEvent("background.task") | ||
| handleBackgroundEvent(payload: { id: number }) { | ||
| this.eventCount++; | ||
| console.log(`[@nestjs/event-emitter] Event #${this.eventCount} received`); | ||
|
|
||
| Sentry.addBreadcrumb({ | ||
| category: "event-job", | ||
| message: `Event handler #${this.eventCount} executed`, | ||
| level: "info", | ||
| }); | ||
|
|
||
| console.log(`[@nestjs/event-emitter] Event #${this.eventCount} done`); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { Injectable, OnModuleInit } from "@nestjs/common"; | ||
| import { Task, TaskHandler } from "nestjs-graphile-worker"; | ||
| import { WorkerService } from "nestjs-graphile-worker"; | ||
| import * as Sentry from "@sentry/nestjs"; | ||
|
|
||
| /** | ||
| * Case 4: nestjs-graphile-worker | ||
| * Graphile worker tasks run outside HTTP request context on the default isolation scope. | ||
| */ | ||
| @Injectable() | ||
| @Task("background-graphile-task") | ||
| export class GraphileJobHandler { | ||
| private taskCount = 0; | ||
|
|
||
| @TaskHandler() | ||
| async handler(payload: { id: number }) { | ||
| this.taskCount++; | ||
| console.log( | ||
| `[nestjs-graphile-worker] Task #${this.taskCount} running` | ||
| ); | ||
|
|
||
| Sentry.addBreadcrumb({ | ||
| category: "graphile-job", | ||
| message: `Graphile task #${this.taskCount} executed`, | ||
| level: "info", | ||
| }); | ||
|
|
||
| console.log( | ||
| `[nestjs-graphile-worker] Task #${this.taskCount} done` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Service that periodically adds tasks to the graphile-worker queue. | ||
| */ | ||
| @Injectable() | ||
| export class GraphileJobProducer implements OnModuleInit { | ||
| private taskCount = 0; | ||
|
|
||
| constructor(private readonly workerService: WorkerService) {} | ||
|
|
||
| async onModuleInit() { | ||
| // Start the graphile-worker runner so it actually processes tasks | ||
| this.workerService.run().catch((err) => { | ||
| console.error("[nestjs-graphile-worker] Runner error:", err); | ||
| }); | ||
|
|
||
| setInterval(async () => { | ||
| this.taskCount++; | ||
| try { | ||
| await this.workerService.addJob("background-graphile-task", { | ||
| id: this.taskCount, | ||
| }); | ||
| console.log( | ||
| `[nestjs-graphile-worker] Added task #${this.taskCount} to queue` | ||
| ); | ||
| } catch (e) { | ||
| // Silently ignore if worker service isn't ready | ||
| } | ||
| }, 6000); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BullMQ connection silently ignores invalid
urlpropertyMedium Severity
The
BullModule.forRoot({ connection: { url: process.env.REDIS_URL } })call passes aurlproperty in the connection options, but ioredis'sRedisOptionsdoes not support aurlproperty (the feature request was explicitly rejected in ioredis#871). Theurlfield is silently ignored, and ioredis defaults tolocalhost:6379. This means the BullMQ test case only works by coincidence when Redis happens to be on localhost, and will silently connect to the wrong server for any otherREDIS_URLvalue. The connection URL needs to be parsed intohost/port/passwordfields or an ioredis instance created from the URL string.