-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
fix(HTTP/XhrBackend): correctly set the status code on errors #9355
Conversation
@@ -181,14 +181,30 @@ export function main() { | |||
existingXHRs[0].dispatchEvent('error'); | |||
})); | |||
|
|||
it('should set the status and status code on error', |
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.
"status and statusText"
}); | ||
const xhr = existingXHRs[0]; | ||
xhr.setStatusCode(0); | ||
xhr.setStatusText(''); |
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 that status=0 and statusText='' are common values for CORS related errors. otherwise we'll wonder what these are.
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.
Not sure if we really care about the given values ? I'll add the comment anyway
please see comments above. otherwise lgtm |
thanks for tge review. I have addressed your comment and will merge. |
fixes angular#9329 fixes angular/http#54
…r#9355) fixes angular#9329 fixes angular/http#54
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
fixes #9329
fixes angular/http#54
@robwormald @jeffbcross could one of you please review ? Thanks