test(NODE-7493): enable source maps and add nyc-free debug scripts by seanrmilligan · Pull Request #4947 · mongodb/node-mongodb-native

seanrmilligan · 2026-05-27T20:40:20Z

Description

1. Accurate interactive debugging (stepping)

check:unit / check:test run under nyc, which instruments src/**/*.ts on the fly. When debugging, the inspector then steps through that Istanbul-rewritten code and lands on comments/whitespace instead of real source lines. This adds nyc-free debug entrypoints so stepping lands on the original TypeScript:

mocha:debug — generic helper: npm run mocha:debug -- <mocha args>
check:unit:debug — unit tests
check:test:debug — integration tests

They drop nyc and pass --inspect --enable-source-maps --no-experimental-strip-types directly. (Source maps for stepping come from ts-node's inline maps regardless; the only fix needed was removing the coverage instrumentation layer.)

2. Correct stack-trace line numbers

Adds --enable-source-maps across the mocha configs so V8 natively reports TypeScript line numbers in Error.stack. test/unit/source_map.test.ts guards this — it nulls Error.prepareStackTrace to assert V8-native mapping is in effect.

Simplification

Removes the redundant explicit source-map-support registration (the inline .install() in configuration.ts). ts-node's bundled @cspotcode/source-map-support already maps both ts-node-compiled src and the inline-source-mapped driver bundle.

Usage

npm run check:unit:debug -- -g "pattern"
npm run check:test:debug -- -g "pattern"   # requires a running server + `source mo-expansion.sh`
npm run mocha:debug -- --config test/manual/mocharc.js test/manual/tls_support.test.ts

Attach a debugger via node inspect 127.0.0.1:9229.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Enables Node.js native source map support across the test runners so that stack traces from compiled TypeScript test files report original .ts line numbers. Adds a demonstration/verification test for the behavior.

Changes:

Adds --enable-source-maps to the node-option array in all mocha config files.
Enables sourceMap: true in test/tsconfig.json so emitted JS has source maps.
Adds a new unit test that verifies V8 natively maps stack frames back to TS source lines.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
.mocharc.js	Adds `enable-source-maps` to node-options for default test runs.
test/mocha_mongodb.js	Adds `enable-source-maps` for integration test runs.
test/mocha_lambda.js	Adds `enable-source-maps` for lambda test runs.
test/manual/mocharc.js	Adds `enable-source-maps` for manual test runs.
test/tsconfig.json	Enables `sourceMap` emission for tests.
test/unit/sourcemap_demo.test.ts	New test verifying V8 reports TS line numbers in stack frames.
package.json	Appends `build:runtime-barrel` to bundled check scripts using `;`.

Pass --enable-source-maps to Node for all mocha configs so V8 resolves inline source maps emitted by ts-node, making the debugger show correct TypeScript file/line locations. Also add sourceMap: true to test/tsconfig.json and ensure bundled test runs reset test/mongodb.ts so stale VM-context state does not affect subsequent debug sessions.

PavelSafronov

One of the Acceptance Criteria is to verify hostMatchesWildcards debugging. This can be done manually (test locally, add the repro steps to the PR description, I'll verify on my side), or maybe we can update sourcemap.test.ts test to import and use hostMatchesWildcards and verify that the debugger experience is improved. Up to you how we verify this.

tadjik1

thanks @seanrmilligan, the approach looks excellent, great idea to use native source maps! I've left few comments, please let me know what you think. in addition to that - there are some merge conflicts, can you please take a look?

nbbeeken · 2026-06-04T14:57:18Z

Screen.Recording.2026-06-04.at.10.53.51.AM.mov

The changes here don't fix the issue and removing the source-map-support package has added the problem to the integration tests

nbbeeken

I think we can solve this in two ways, one simple, one probably a large-ish undertaking.

Just add the source-map-support import to a mocha "require"-ed file that the unit tests use and maybe even share it with the int tests to de-dupe. This is the only way I know that the sourcemaps will work with ts-node
Remove ts-node, its no longer needed with the builtin support typescript in Node.js, if we go that direction then your "native" sourcemap flag that's being added here would work! but I fear that the tests and how they are formatted (like import paths and such) are not going to work without some heavy lifting
- I would still say this is the right way to do things but schedule it in the future with proper estimation

…dled

Use `VERBOSE=1 npm run check:unit -- -g "Source maps"` to run

seanrmilligan · 2026-06-04T15:28:03Z

Screen.Recording.2026-06-04.at.10.53.51.AM.mov
The changes here don't fix the issue and removing the source-map-support package has added the problem to the integration tests

Could you share the test that's exercising the path?

seanrmilligan · 2026-06-04T15:34:08Z

I think we can solve this in two ways, one simple, one probably a large-ish undertaking.

Just add the source-map-support import to a mocha "require"-ed file that the unit tests use and maybe even share it with the int tests to de-dupe. This is the only way I know that the sourcemaps will work with ts-node

Remove ts-node, its no longer needed with the builtin support typescript in Node.js, if we go that direction then your "native" sourcemap flag that's being added here would work! but I fear that the tests and how they are formatted (like import paths and such) are not going to work without some heavy lifting

I would still say this is the right way to do things but schedule it in the future with proper estimation

Re: second bullet

I looked into using native Node.js TypeScript debugging symbol support but it seemed infeasible.

minor reason: The feature was added in v22 with a flag, v22.18 without a flag. We still support older versions of Node.js. We could overcome this by conditionally splitting how we treat the build by version in .mocharc.js and test/mocha_mongodb.js
major reason: The disparate ways Node.js recognizes and handles CJS and ESM files has been smoothed over for us by the ts-node package. Backing out that package to embrace native support would require fixing things in quite a few places, like how imports/requires work. Some of those are lazy-loaded and to re-implement them would require more work than this bug is sized for.

nbbeeken · 2026-06-04T15:37:57Z

Yea agreed, to your minor point we don't go back to test older versions of Node within a major so any 22 testing is going to be after that feature release, but even if it was before that feature release the env var NODE_OPTIONS would be a way to enable the expiremental parsing wherever you need it.

But yea major point, the TS in the test files is not erasableSyntaxOnly and verbatimModuleSyntax compliant, if the work was done to get the tests building with those flags then you could use node's builtin support.

nbbeeken · 2026-06-04T15:40:00Z

Could you share the test that's exercising the path?

It's not a specific test, I'm just running npm run check:unit and I know there's something that eventually hits the cursor code at some point 😅 just because of it's centrality to the driver's API. This would reproduce on any test, you just have to reach a code comment somewhere in src

Looking at the video, its probably the second test under the "Collation" suite, whatever hasn't printed yet

…setup - add mocha:debug + check:unit:debug + check:test:debug, which run mocha without nyc/Istanbul so the interactive debugger steps onto real source lines instead of instrumented code (comments/whitespace) - drop the redundant explicit source-map-support (source_map.cjs + the inline install in configuration.ts); ts-node's bundled @cspotcode/source-map-support already maps ts-node-compiled src and the inline-mapped driver bundle - keep --enable-source-maps across the mocha configs (guarded by source_map.test.ts) for correct V8-native stack-trace line numbers

…p.test.ts Read the probe site's line from the test file at runtime instead of hard-coding TS_SOURCE_LINE, so the assertion can't drift when lines are added or removed above it.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

nbbeeken · 2026-06-08T18:50:05Z

    "check:csfle": "npm run build:bundle && nyc mocha --config test/mocha_mongodb.js test/integration/client-side-encryption",
    "check:snappy": "npm run build:bundle && nyc mocha test/unit/assorted/snappy.test.js",
    "check:x509": "npm run build:bundle && nyc mocha test/manual/x509_auth.test.ts",
+    "mocha:debug": "node --inspect --enable-source-maps --no-experimental-strip-types ./node_modules/mocha/bin/mocha.js",


This comment has merit (insane code suggestion tho)

it's not really common to directly reference paths into node_modules especially into specific packages as mocha can move the location of it's "bin" at will.

tsc is being brought in by two packages and we are forced to reference it directly to make sure we're using the one we depend on in our own package.json (perhaps this has been fixed in more recent versions of npm / tsd

Plus mocha takes flags directly that you would like to pass to Node:

mocha --inspect -n enable-source-maps -n no-experimental-strip-types

But in the usages of this script you're still utilizing the config files that set these flags, so wouldn't inspect just be the new one here?

FFTR, same sentiment, c8 will fix.

PavelSafronov · 2026-06-09T21:32:26Z

+  const saved = Error.prepareStackTrace;
+  // Null → V8 uses its built-in formatter (honours --enable-source-maps).
+  Error.prepareStackTrace = undefined as unknown as typeof Error.prepareStackTrace;
+  const raw = err.stack ?? ''; // triggers V8 formatting with no hook
+  Error.prepareStackTrace = saved;


The first half of the comment makes sense. The addition of the try/catch is not necessary: we want that error to bubble up.

nbbeeken

btw, there are merge conflicts 🙂

nbbeeken · 2026-06-08T18:39:59Z

-// eslint-disable-next-line @typescript-eslint/no-require-imports
-require('source-map-support').install({
-  hookRequire: true
-});


Doesn't this make integration tests not debug with the correct lines? I know we didn't discover how to replicate this for unit tests but does it make sense to remove for int?

FFTR, we know the real fix is c8 and it's on the horizon

nbbeeken · 2026-06-08T18:50:05Z

    "check:csfle": "npm run build:bundle && nyc mocha --config test/mocha_mongodb.js test/integration/client-side-encryption",
    "check:snappy": "npm run build:bundle && nyc mocha test/unit/assorted/snappy.test.js",
    "check:x509": "npm run build:bundle && nyc mocha test/manual/x509_auth.test.ts",
+    "mocha:debug": "node --inspect --enable-source-maps --no-experimental-strip-types ./node_modules/mocha/bin/mocha.js",


This comment has merit (insane code suggestion tho)

it's not really common to directly reference paths into node_modules especially into specific packages as mocha can move the location of it's "bin" at will.

tsc is being brought in by two packages and we are forced to reference it directly to make sure we're using the one we depend on in our own package.json (perhaps this has been fixed in more recent versions of npm / tsd

Plus mocha takes flags directly that you would like to pass to Node:

mocha --inspect -n enable-source-maps -n no-experimental-strip-types

But in the usages of this script you're still utilizing the config files that set these flags, so wouldn't inspect just be the new one here?

…re self-contained The :debug scripts previously skipped build:bundle, requiring a prior check:* run. Prepend it in mocha:debug (inherited by check:unit:debug and check:test:debug) to match the auto-build behavior of the check:* scripts.

PavelSafronov

Works on my machine!

Tested with bundled and unbundled tests, both unit and integ. Everything is looking good.

PavelSafronov

Just tried debugging unit tests with Node.js v20, and process crashes. :(

PavelSafronov · 2026-06-09T22:18:01Z

    "check:csfle": "npm run build:bundle && nyc mocha --config test/mocha_mongodb.js test/integration/client-side-encryption",
    "check:snappy": "npm run build:bundle && nyc mocha test/unit/assorted/snappy.test.js",
    "check:x509": "npm run build:bundle && nyc mocha test/manual/x509_auth.test.ts",
+    "mocha:debug": "npm run build:bundle && node --inspect --enable-source-maps --no-experimental-strip-types ./node_modules/mocha/bin/mocha.js",


The flag --no-experimental-strip-types doesn't exist in Node.js v20, so this fails in that environment:

15:11:52 ~/code/node-mongodb-native $ node --version v20.20.2 15:11:57 ~/code/node-mongodb-native $ npm run check:unit:debug > mongodb@7.2.0 check:unit:debug > npm run mocha:debug -- test/unit > mongodb@7.2.0 mocha:debug > npm run build:bundle && node --inspect --enable-source-maps --no-experimental-strip-types ./node_modules/mocha/bin/mocha.js test/unit > mongodb@7.2.0 build:bundle > npm run bundle:driver && npm run bundle:types && npm run build:runtime-barrel > mongodb@7.2.0 bundle:driver > node etc/bundle-driver.mjs test/tools/runner/bundle/driver-bundle.js 3.7mb ⚠️ ⚡ Done in 38ms ✓ Driver bundle created at /Users/pavel.safronov/code/node-mongodb-native/test/tools/runner/bundle/driver-bundle.js > mongodb@7.2.0 bundle:types > npx tsc --project ./tsconfig.json --declaration --emitDeclarationOnly --declarationDir test/tools/runner/bundle/types > mongodb@7.2.0 build:runtime-barrel > node etc/build-runtime-barrel.mjs ✓ /Users/pavel.safronov/code/node-mongodb-native/test/mongodb.ts now re-exports from ./mongodb_all node: bad option: --no-experimental-strip-types Result=9

PavelSafronov · 2026-06-09T22:19:55Z

Nit: source-map-support is no longer used, remove?

seanrmilligan force-pushed the sean.milligan/source-maps branch from 0eac018 to 5e04b2e Compare May 27, 2026 21:48

seanrmilligan marked this pull request as ready for review May 27, 2026 21:56

seanrmilligan requested a review from a team as a code owner May 27, 2026 21:56

Copilot AI review requested due to automatic review settings May 27, 2026 21:56

Copilot AI reviewed May 27, 2026

View reviewed changes

seanrmilligan force-pushed the sean.milligan/source-maps branch from 5e04b2e to 228897f Compare May 27, 2026 22:02

PavelSafronov reviewed May 28, 2026

View reviewed changes

Comment thread package.json Outdated

PavelSafronov reviewed May 28, 2026

View reviewed changes

Comment thread test/unit/source_map.test.ts Outdated

tadjik1 self-assigned this Jun 2, 2026

tadjik1 added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jun 2, 2026

tadjik1 and others added 3 commits June 3, 2026 18:52

Merge branch 'main' into sean.milligan/source-maps

9f81852

test(NODE-7493): enable source maps for accurate unit test debugging

340248e

Rename source map test

d5fbddd

tadjik1 reviewed Jun 4, 2026

View reviewed changes

Comment thread test/unit/source_map.test.ts Outdated

tadjik1 reviewed Jun 4, 2026

View reviewed changes

Comment thread test/unit/source_map.test.ts Outdated

tadjik1 requested changes Jun 4, 2026

View reviewed changes

nbbeeken requested changes Jun 4, 2026

View reviewed changes

Sean Milligan added 3 commits June 4, 2026 08:15

Remove building the barrel from check:unit-bundled and check:test-bun…

130a107

…dled

fix: typo

54b538c

Guard console logs with VERBOSE variable

d017df9

Use `VERBOSE=1 npm run check:unit -- -g "Source maps"` to run

tadjik1 reviewed Jun 5, 2026

View reviewed changes

Comment thread test/manual/mocharc.js

Sean Milligan added 3 commits June 5, 2026 14:15

wip

f0ab685

wip

a1be2a8

seanrmilligan changed the title ~~test(NODE-7493): Enable source maps for accurate test debugging~~ test(NODE-7493): enable source maps and add nyc-free debug scripts Jun 8, 2026

test(NODE-7493): compute expected source line at runtime in source_ma…

9572717

…p.test.ts Read the probe site's line from the test file at runtime instead of hard-coding TS_SOURCE_LINE, so the assertion can't drift when lines are added or removed above it.

seanrmilligan requested review from PavelSafronov, Copilot, nbbeeken and tadjik1 June 8, 2026 18:13

Copilot started reviewing on behalf of seanrmilligan June 8, 2026 18:13 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

nbbeeken requested changes Jun 8, 2026

View reviewed changes

PavelSafronov reviewed Jun 8, 2026

View reviewed changes

Comment thread package.json

Sean Milligan added 3 commits June 8, 2026 13:45

Merge branch 'main' into sean.milligan/source-maps

5c8b796

remove source map test

a395f48

PavelSafronov approved these changes Jun 8, 2026

View reviewed changes

nbbeeken approved these changes Jun 8, 2026

View reviewed changes

PavelSafronov requested changes Jun 9, 2026

View reviewed changes

Conversation

seanrmilligan commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PavelSafronov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tadjik1 left a comment

Choose a reason for hiding this comment

Uh oh!

nbbeeken commented Jun 4, 2026

Uh oh!

nbbeeken left a comment

Choose a reason for hiding this comment

Uh oh!

seanrmilligan commented Jun 4, 2026

Uh oh!

seanrmilligan commented Jun 4, 2026

Uh oh!

nbbeeken commented Jun 4, 2026

Uh oh!

nbbeeken commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

nbbeeken Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbbeeken Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

PavelSafronov Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

nbbeeken left a comment

Choose a reason for hiding this comment

Uh oh!

nbbeeken Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

nbbeeken Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nbbeeken Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PavelSafronov left a comment

Choose a reason for hiding this comment

Uh oh!

PavelSafronov left a comment

Choose a reason for hiding this comment

Uh oh!

PavelSafronov Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

seanrmilligan commented May 27, 2026 •

edited

Loading

nbbeeken commented Jun 4, 2026 •

edited

Loading

nbbeeken Jun 8, 2026 •

edited

Loading

nbbeeken Jun 8, 2026 •

edited

Loading