fix(mcp-server): blame the missing dependency, not the configured path - #1834
Open
Scra3 wants to merge 1 commit into
Open
fix(mcp-server): blame the missing dependency, not the configured path#1834Scra3 wants to merge 1 commit into
Scra3 wants to merge 1 commit into
Conversation
A storage module whose own dependency is not installed reported FOREST_MCP_UPLOAD_STORAGE_MODULE "…" was not found, sending the operator to check a path that was correct. Node raises MODULE_NOT_FOUND for that case too, naming the dependency as the subject and carrying the module's own path in the require stack, so matching the resolved path anywhere in the message caught it. Matching the quoted name separates them: the entry point failing reads Cannot find module '<resolved>', a missing dependency reads Cannot find module '<dependency>'. Found writing the S3 example for a customer: `npm i @aws-sdk/client-s3` forgotten is the first thing anyone configuring this variable will hit.
|
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
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
1 participant
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.
Found while writing the S3 storage example for a customer about to configure
FOREST_MCP_UPLOAD_STORAGE_MODULE: the module was on disk and correct, and the server said it was not there.The real cause was
@aws-sdk/client-s3not being installed. Node raisesMODULE_NOT_FOUNDfor that too, naming the dependency as the subject and carrying the module's own path in the require stack — so the old test,message.includes(resolved), matched on the stack line and reported the wrong diagnosis.Matching the quoted name separates the two cases:
Cannot find module '<resolved>'was not found (resolved to …)Cannot find module '<dependency>'failed while loading: Cannot find module '<dependency>'Verified in plain node against the built
dist, both directions — the Jest resolver rewrites this message, so a test alone would not have proven it. Pinned byblames the dependency, not the path, when the module requires something missing, alongside the existing test for a genuinely absent path.npm i @aws-sdk/client-s3forgotten is the first thing anyone setting this variable will hit, and it was the one mistake the error actively mislabelled.🤖 Generated with Claude Code
Note
Fix
loadFileUploadsto distinguish missing module from missing dependency inMODULE_NOT_FOUNDerrorsWhen
require()throws aMODULE_NOT_FOUNDerror, it can mean either the target module itself is absent or a dependency of that module is missing. Previously, the function treated both cases as 'module not found'. It now checks whether the error message contains the resolved path wrapped in single quotes; only that case is classified as 'not found', while missing-dependency errors are classified as 'failed while loading'.Macroscope summarized 701d045.