Skip to content
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

Add sourceURL to list of error keys. #71

Closed
wants to merge 2 commits into from

Conversation

koddsson
Copy link
Member

It looks like Safari has some property on errors that I can't find anything about online. Adding it to the list so that tests in chai/chaijs pass.

See chaijs/chai#1546 for a PR that's failing on Safari. Direct link to build: https://github.com/chaijs/chai/actions/runs/6610600744/job/17952929090?pr=1546.

Test failures:

test/globalErr.js:

 ❌ globalErr > should throw if object val's props are not included in error object
      AssertionError: expected 'expected AssertionError: cat { source…' to match /expected AssertionError: cat to have property \'text\'/
        at AssertionError (chai.js:294:10)
        at chai.js:1545:29
        at assertMatch (chai.js:2645:14)
        at chai.js:1708:30
        at globalErr (test/bootstrap/index.js:49:62)
        at test/globalErr.js:165:8

 ❌ globalErr > skipStackTest > falsey > should throw if "Getter" is in the stack trace
      AssertionError: Expected an error
        at AssertionError (chai.js:294:10)
        at globalErr (test/bootstrap/index.js:58:32)
        at test/globalErr.js:241:12

 ❌ globalErr > skipStackTest > falsey > should throw if "Wrapper" is in the stack trace
      AssertionError: Expected an error
        at AssertionError (chai.js:294:10)
        at globalErr (test/bootstrap/index.js:58:32)
        at test/globalErr.js:249:12

 ❌ globalErr > skipStackTest > falsey > should throw if "assert" is in the stack trace
      AssertionError: Expected an error
        at AssertionError (chai.js:294:10)
        at globalErr (test/bootstrap/index.js:58:32)
        at test/globalErr.js:257:12

@koddsson koddsson marked this pull request as ready for review October 23, 2023 08:52
@@ -12,6 +12,7 @@ const errorKeys = [
'columnNumber',
'number',
'description',
'sourceURL' // A property that seems to be only in Safari
Copy link
Member

@keithamus keithamus Oct 23, 2023

Choose a reason for hiding this comment

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

Can we add some tests within Loupe for this?

@koddsson
Copy link
Member Author

Decided to drop this as this has been a problem for some time and I think we should actually try to normalize the stacktrace across all browsers but that work is a bit more than just this PR.

@koddsson koddsson closed this Oct 23, 2023
@koddsson koddsson deleted the ignore-source-url-property branch October 23, 2023 12:49
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