-
Notifications
You must be signed in to change notification settings - Fork 225
Catch XHR errors #222
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
Catch XHR errors #222
Conversation
source-map-support.js
Outdated
var xhr = new XMLHttpRequest(); | ||
xhr.open('GET', path, false); | ||
xhr.send(null); | ||
var contents = null |
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.
Don't redeclare contents
here.
source-map-support.js
Outdated
try { | ||
// Use SJAX if we are in the browser | ||
var xhr = new XMLHttpRequest(); | ||
xhr.open('GET', path, false); |
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.
Please add a comment like
// Note: synchronous XHR
It's very weird so a comment would alert readers that it is intended.
or you could use /** async */ false
in the call.
I did not change any of the original code in this PR, just added the try/catch. But of course I can have a look at it. |
68dcad4
to
b024dfe
Compare
@johnjbarton fixed. |
Looks great! Thanks, added a commit to refactor the try/catch, will merge when CI is green 👍 |
Thanks! 👍 |
@LinusU When are you planning to make a release that will contain this commit? |
Thx ❤️ released as 0.5.9 🚀 edit: @duydnguyen07 yes |
When the XHR request throws an error (e.g. in case of CORS issues), it is now catched properly. Needed to fix weird Karma/Jasmine issues in Angular CLI projects.
Closes angular/angular-cli#7296.