-
Notifications
You must be signed in to change notification settings - Fork 146
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
Feature request: improve regex in stack trace parsing #2121
Comments
I would like to contribute. Will submit a PR with the regex and unit tests. |
Hi @karthikeyanjp, thank you for reaching out. I'll assign the issue to you and help merge the PR once ready. |
Referring to format of error locations as mentioned in https://nodejs.org/api/errors.html#errorstack & https://v8.dev/docs/stack-trace-api , we can support the following pattern:
For pattern (3), the current & above proposed regex will extact I am proposing the regex Submitted PR #2194 . @dreamorosi please let me know your thoughts on the new regex. |
This issue is now closed. Please be mindful that future comments are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so. |
This is now released under v2.0.3 version! |
Use case
When formatting an error using the Logger utility I want to get the location of the error in my logs. To do so, the Logger utility parses the stack trace of an error and extracts the relevant info.
For example, the following stack trace:
yields this location:
/home/foo/bar/file-that-threw-the-error.ts:22:5
.At the moment we are using a regex expression (
/\((.*):(\d+):(\d+)\)\\?$/
) that has a couple of potential issues that could lead to unexpected behavior:.*
part of the regex uses a greedy quantifier. This means it will match as many characters as possible, which could lead to unexpected results if there are multiple colons in the string. For example, in the string "(foo:bar:1:2)", the.*
part will match "foo:bar", not just "foo".\\?
part of the regex matches an optional backslash at the end of the string. However, if since we're trying to match a single backslash, we only need to escape it once, not twice.Solution/User Experience
The API/DX for the logger should remain the same and the change should be backwards compatible.
We should however improve the expression and change it to
/\((.*?):(\d+):(\d+)\)\$?$/
, this would:.*
->.*?
\\?$
->\?$
Additionally, we should review the unit tests of the method associated with this expression and make sure they're exhaustive enough.
Alternative solutions
No response
Acknowledgment
Future readers
Please react with 👍 and your use case to help us understand customer demand.
The text was updated successfully, but these errors were encountered: