Repository navigation
Fix example servers crashing at startup on Node older than 20.11 - #787
Merged
Merged
Conversation
The example servers located their dist directory with import.meta.filename / import.meta.dirname, which only exist from Node 20.11. On older versions (18.x, 20.0 to 20.10) the module threw "TypeError: Cannot read properties of undefined (reading 'endsWith')" at load, before the server could answer `initialize`, even though the packages are documented as supporting Node 20+. - Derive the path from import.meta.url with fileURLToPath, the idiom lazy-auth-server already uses, in the 21 affected example servers and in the quickstart (docs/quickstart.md resynced). - Declare engines.node ">=20" in the published example packages and record it in package-lock.json. - CI: after the MCPB smoke test, run the same pdf-server bundle on Node 20.0.0 so a regression fails the build.
Contributor
PreviewPreview deployments for this PR have been cleaned up. |
@modelcontextprotocol/ext-apps
@modelcontextprotocol/server-basic-preact
@modelcontextprotocol/server-basic-react
@modelcontextprotocol/server-basic-solid
@modelcontextprotocol/server-basic-svelte
@modelcontextprotocol/server-basic-vanillajs
@modelcontextprotocol/server-basic-vue
@modelcontextprotocol/server-budget-allocator
@modelcontextprotocol/server-cohort-heatmap
@modelcontextprotocol/server-customer-segmentation
@modelcontextprotocol/server-debug
@modelcontextprotocol/server-lazy-auth
@modelcontextprotocol/server-map
@modelcontextprotocol/server-pdf
@modelcontextprotocol/server-scenario-modeler
@modelcontextprotocol/server-shadertoy
@modelcontextprotocol/server-sheet-music
@modelcontextprotocol/server-system-monitor
@modelcontextprotocol/server-threejs
@modelcontextprotocol/server-transcript
@modelcontextprotocol/server-video-resource
@modelcontextprotocol/server-wiki-explorer
commit: |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The example servers compute their
distdirectory fromimport.meta.filename/import.meta.dirname, which only exist from Node 20.11. On Node 18.x and 20.0 to 20.10 the module throws at load, before the server can answerinitialize:This affects the published packages, including
@modelcontextprotocol/server-pdf1.7.5 and 2.0.0, while the README and quickstart say Node 20+ is supported.Changes
examples/*/server.ts): derive the path fromimport.meta.urlwithfileURLToPath, the idiomexamples/lazy-auth-server/server.tsalready uses. No behaviour change on Node 20.11+.examples/quickstart/server.ts): same fix;docs/quickstart.mdresynced withnpm run sync:snippets.engines.node: ">=20"added to the 21 published example packages (the ones with abin), matching the root package.package-lock.jsongains only those 21enginesentries (63 added lines, nothing removed, no dependency changes).Testing
Everything below was run in a clean Linux arm64 container, with the build done on Node 22 as in CI and official Node tarballs (SHA-256 checked against
SHASUMS256.txt) used to run the built servers.npm ci,npm run build,npm run examples:build,npm run generate:schemas(no diff),npm run sync:snippets(no diff after the resync),npm test(438 pass, 2 skip, 0 fail),npm run prettier,node scripts/check-versions.mjs, the TypeDoc link validation and the lockfile registry check all pass.Each built server was started with
node dist/index.js --stdio, sentinitialize, and then asked for itsui://resource (which is what reads from the computeddistdirectory).lazy-auth-serveris HTTP-only, so it was probed over HTTP instead.main(before)The "before" column counts the 21 servers that used the pattern (
lazy-auth-serverwas already fine). On Node 18.20.4 the one remaining failure iswiki-explorer-server, whosecheerio/undicidependency needs theFileglobal; Node 18 is outside the declared range.For pdf-server specifically:
node build-mcpb.mjs, unzipped, and the exact CI smoke command (node .mcpb-smoke/dist/index.js --stdio < /dev/null, thengrep -q Ready) passes on Node 22 and on Node 20.0.0.mainfails on Node 20.0.0 with theTypeErrorabove, so the new CI step does detect the bug.display_pdfon a local PDF succeeds on 18.20.4, 20.0.0, 20.10.0, 20.11.0 and 22.The new CI step itself (the
actions/setup-nodeswitch to 20.0.0) can only be exercised by CI on this PR.Notes on the
enginesfloor>=20is what was verified: every published example starts and serves its UI on Node 20.0.0. Two dependencies declare a higher floor than that, which npm reports as anEBADENGINEwarning at install time:pdfjs-distdeclares>=20.16.0 || >=22.3.0. On older Node it logs a warning aboutprocess.getBuiltinModuleto stderr; pdf-server still starts and opens PDFs.cheeriodeclares>=20.18.1(used by wiki-explorer-server), which starts and answers tool calls on 20.0.0.If we would rather not claim more than those dependencies do, the alternative is to raise the floor for those two packages only. Happy to change it either way.
Follow-ups
plugins/mcp-apps/skills/add-app-to-server/SKILL.mdandplugins/mcp-apps/skills/convert-web-app/SKILL.mdstill showimport.meta.dirnamein their snippets. They are fragments without imports, so they are left for a separate docs change.examples/pdf-server/server.test.tsusesimport.meta.dirname, but it only runs under Bun.