-
-
Notifications
You must be signed in to change notification settings - Fork 440
Add original path to error messages #363
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
base: master
Are you sure you want to change the base?
Conversation
b25a26b
to
3f25805
Compare
src/index.ts
Outdated
*/ | ||
function errorMessage(message: string, originalPath: string | undefined) { | ||
if (originalPath) { | ||
return `${message}: ${originalPath}; visit ${DEBUG_URL} for more info`; |
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.
Not tied to this format, happy to get other opinions. Another PR used ${originalPath}: ${message}
format instead.
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.
LGTM!
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.
LGTM!
Can we move forward and make a release for this? this really helps a lot with migrating from Express 4 to Express 5 |
I think it is time to land it with new realease |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #363 +/- ##
===========================================
- Coverage 100.00% 99.21% -0.79%
===========================================
Files 1 1
Lines 668 381 -287
Branches 151 132 -19
===========================================
- Hits 668 378 -290
- Misses 0 3 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Refactored and added a test to reduce stack size for pillarjs/router#158. |
src/index.spec.ts
Outdated
} catch (error) { | ||
const stack = (error as Error).stack | ||
?.split("\n") | ||
.slice(0, 6) |
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.
This still might not be short enough to get the line when combined with the router stack, but further optimization to reduce function calls starts to affect readability of code.
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.
LGTM, for posterity, why did you stop using generators?
Easiest way reduce the error stack size, otherwise it has 2-3 lines per generator. |
Closes #336, #358, #356, and #354.
Adds
originalPath
toTokenData
, which will be used (when available) for the error messages. It's already added to any methods that input a string.