Skip to content

Add a helpful error message when pathRegexp fails #158

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
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JL102
Copy link

@JL102 JL102 commented Apr 7, 2025

I encountered this issue when migrating from Express v4 to v5. When I had route strings like '/*', it failed, but the error message I got was quite vague (but worse, it does NOT point to my violating line of code or show my violating string):

Error: TypeError: Missing parameter name at 2: https://git.new/pathToRegexpError
    at name (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:156:12)
    at lexer (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:173:6)
    at lexer.next (<anonymous>)
    at Iter.peek (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:190:6)
    at Iter.tryConsume (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:196:37)
    at Iter.text (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:215:6)
    at consume (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:286:29)
    at parse (/workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:322:2)
    at /workspaces/scoutradioz/primary/node_modules/path-to-regexp/src/index.ts:507:48
    at Array.map (<anonymous>)

Adding this try-catch will still spit out the error message provided by path-to-regexp, which points to their documentation where we can read more about the issue, but more importantly, the new error message/stack will contain the violating string and line of source code.

Error: The path route '/*' could not be parsed. Check the above error message for details.
    at matcher (/workspaces/scoutradioz/primary/node_modules/router/lib/layer.js:96:10)
    at new Layer (/workspaces/scoutradioz/primary/node_modules/router/lib/layer.js:99:61)
    at Function.route (/workspaces/scoutradioz/primary/node_modules/router/index.js:428:17)
    at Function.Router.<computed> [as get] (/workspaces/scoutradioz/primary/node_modules/router/index.js:447:24)
    at Object.<anonymous> (/workspaces/scoutradioz/primary/src/routes/scouting.ts:23:8)
    at Module._compile (node:internal/modules/cjs/loader:1565:14)
    at Module.m._compile (/workspaces/scoutradioz/node_modules/ts-node/src/index.ts:1618:23)
    at node:internal/modules/cjs/loader:1708:10
    at Object.require.extensions.<computed> [as .ts] (/workspaces/scoutradioz/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1318:32)

});
}
catch (err) {
console.error(err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d prefer to merge the message you’re suggesting with the error message, rather than logging the error to the console and then throwing a new one

@bjohansebas
Copy link
Contributor

Look pillarjs/path-to-regexp#363, I think it's better to do this in path-to-regexp than in the router

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants