Skip to content

Fix parsing of error stacks where url paths contain @ symbols #42

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

Merged
merged 1 commit into from
May 29, 2018
Merged

Fix parsing of error stacks where url paths contain @ symbols #42

merged 1 commit into from
May 29, 2018

Conversation

bengourley
Copy link
Contributor

When the URL path of a stackframe contains @, the parseFFOrSafari() function returns an incomplete fileName value.

e.g. given the string:

"who@http://localhost:5000/misc/@stuff/foo.js:3:9"

expected fileName is "http://localhost:5000/misc/@stuff/foo.js"
actual fileName is "stuff/foo.js"

Description

Calling .split('@') on the stackframe string split it into an arbitrary number of chunks depending on whether there was an @ in the URL or in the method name.

I created a rather gnarly looking regex (after exhausting other options) to extract everything after the first @, unless it was preceded by something like obj["@method"], in which case it will extract everything after the first @ preceded by the enclosing quotes.

Motivation and Context

We're using this module on bugsnag-js and a customer of ours reported an issue with errors coming from URLs containing @ but only in Firefox.

How Has This Been Tested?

Cases have been added to the automated test suite.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • node_modules/.bin/jscs -c .jscsrc error-stack-parser.js passes without errors
  • npm test passes without errors
  • I have read the contribution guidelines
  • I have updated the documentation accordingly (Not deemed necessary for this change)
  • I have added tests to cover my changes

@bengourley bengourley mentioned this pull request May 25, 2018
@bengourley bengourley merged commit 1054ec2 into stacktracejs:master May 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant