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
Detect JS Engine when reporting errors #7708
Detect JS Engine when reporting errors #7708
Conversation
This should save us some time trying to determine what the real browser is, not what the UserAgent is masquerading as.
Also add comments
test/functional/test-error.js
Outdated
// SauceLabs, which runs does not masquerade with UserAgent. | ||
describe.configure().ifIos().run('on iOS', () => { | ||
it.configure().ifSafari().run('detects safari as safari', () => { | ||
expect(detectJsEngineFromStack()).to.equal('Safari'); |
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.
Indent if off in this file.
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.
Fixed.
src/error.js
Outdated
throw new Error('message'); | ||
} | ||
}); | ||
try { |
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.
Why don't we run this on the server side?
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.
Firefox and IE return different results based on how the prototype is constructed.
function Klass() {}
Klass.prototype.t
// Different than
proto = { t };
obj = Object.create(proto);
It'd have to do much more complicated testing if I don't know for certain how the chain is constructed.
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.
K. Please update errortracker.go to append this to the UA string (maybe wrapped in [] or something like that).
test/functional/test-error.js
Outdated
@@ -409,3 +410,40 @@ describes.sandboxed('reportError', {}, () => { | |||
}).to.throw(/_reported_ Error reported incorrectly/); | |||
}); | |||
}); | |||
|
|||
describe.only('detectJsEngineFromStack', () => { |
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.
Can you update the integration test glob to include this (so the tests actually run on SC)?
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.
Done.
* Detect JS Engine when reporting errors This should save us some time trying to determine what the real browser is, not what the UserAgent is masquerading as. * Fix IE detection Also add comments * linting * JSDoc * Add error tests to integrations suite * Fix closure type check
This should save us some time trying to determine what the real browser
is, not what the UserAgent is masquerading as.