fix(common): incorrect error type for XHR errors in TestRequest
#36082
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devversion
force-pushed
the
fix/common-http-testing-type-error-event-properly
branch
6 times, most recently
from
98ec624
to
7ecec02
Compare
devversion
added
area: common/http
action: review
alxhub
approved these changes
Apr 9, 2020
devversion
force-pushed
the
fix/common-http-testing-type-error-event-properly
branch
2 times, most recently
from
cd1d50f
to
6c52c2f
Compare
devversion
force-pushed
the
fix/common-http-testing-type-error-event-properly
branch
from
6c52c2f
to
5acd0f2
Compare
This was referenced May 27, 2020
Closed
devversion
force-pushed
the
fix/common-http-testing-type-error-event-properly
branch
from
5acd0f2
to
58698c6
Compare
pullapprove
bot
requested review from
alxhub,
jelbourn and
pkozlowski-opensource
jelbourn
approved these changes
Jun 19, 2020
devversion
force-pushed
the
fix/common-http-testing-type-error-event-properly
branch
2 times, most recently
from
bffca6e
to
3cac51b
Compare
This comment has been minimized.
This comment has been minimized.
|
You can preview 3e29972 at https://pr36082-3e29972.ngbuilds.io/. |
pullapprove
bot
requested review from
alxhub,
atscott,
dylhunn and
jessicajaniuk
|
@jessicajaniuk I've just resurrected this PR. Addressed feedback etc. I just need one more public API approval. |
devversion
added
target: minor
jessicajaniuk
approved these changes
Nov 18, 2021
|
I'll run a TGP to make sure we're safe in google3 with this change. |
alxhub
approved these changes
Nov 18, 2021
jessicajaniuk
added
action: merge
Currently the `HttpClient` always wraps errors from XHR requests, but the underlying errors are always of type `ProgressEvent`, or don't have a native error if the status code is just indicating failure (e.g. 404). This behavior does not match in the `TestRequest` class provided by `@angular/common/http/testing` where errors are considered being of type `ErrorEvent`. This is incorrect because `ErrorEvent`s provide information for errors in scripts or files which are evaluated. Since the `HttpClient` never evaluates scripts/files, and also since XHR requests clearly are documented to emit `ProgressEvent`'s, we should change the `TestSupport` to retrieve such `ProgressEvent`'s instead of incompatible objects of type `ErrorEvent`. In favor of having a deprecation period, we keep supporting `ErrorEvent` in the `TestRequest.error` signature. Eventually, we can remove this signature in the future. Resources: * https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/error_event * https://developer.mozilla.org/en-US/docs/Web/API/ErrorEvent * https://xhr.spec.whatwg.org/#event-xhr-errpr Related to: angular#34748. DEPRECATED: `TestRequest` from `@angular/common/http/testing` no longer accepts `ErrorEvent` when simulating XHR errors. Instead instances of `ProgressEvent` should be passed, matching with the native browser behavior.
jessicajaniuk
force-pushed
the
fix/common-http-testing-type-error-event-properly
branch
from
3e29972
to
9598e7c
Compare
|
@devversion Can you rebase this and repush? It seems to fail to build in google3 now. |
|
You can preview 9598e7c at https://pr36082-9598e7c.ngbuilds.io/. |
|
The TGP passed. |
jessicajaniuk
removed
the
action: presubmit
|
This PR was merged into the repository by commit 489cf42. |
|
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. |
dimakuba
pushed a commit
to dimakuba/angular
that referenced
this pull request
Dec 28, 2021
…gular#36082) Currently the `HttpClient` always wraps errors from XHR requests, but the underlying errors are always of type `ProgressEvent`, or don't have a native error if the status code is just indicating failure (e.g. 404). This behavior does not match in the `TestRequest` class provided by `@angular/common/http/testing` where errors are considered being of type `ErrorEvent`. This is incorrect because `ErrorEvent`s provide information for errors in scripts or files which are evaluated. Since the `HttpClient` never evaluates scripts/files, and also since XHR requests clearly are documented to emit `ProgressEvent`'s, we should change the `TestSupport` to retrieve such `ProgressEvent`'s instead of incompatible objects of type `ErrorEvent`. In favor of having a deprecation period, we keep supporting `ErrorEvent` in the `TestRequest.error` signature. Eventually, we can remove this signature in the future. Resources: * https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/error_event * https://developer.mozilla.org/en-US/docs/Web/API/ErrorEvent * https://xhr.spec.whatwg.org/#event-xhr-errpr Related to: angular#34748. DEPRECATED: `TestRequest` from `@angular/common/http/testing` no longer accepts `ErrorEvent` when simulating XHR errors. Instead instances of `ProgressEvent` should be passed, matching with the native browser behavior. PR Close angular#36082
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the
HttpClientalways wraps errors from XHR requests, butthe underlying errors are always of type
ProgressEvent, or don't havea native error if the status code is just indicating failure (e.g. 404).
This behavior does not match in the
TestRequestclass provided by@angular/common/http/testingwhere errors are considered beingof type
ErrorEvent. This is incorrect becauseErrorEvents provideinformation for errors in scripts or files which are evaluated. Since
the
HttpClientnever evaluates scripts/files, and also since XHR requestsclearly are documented to emit
ProgressEvent's, we should change theTestSupportto retrieve suchProgressEvent's instead of incompatibleobjects of type
ErrorEvent.Resources:
Related to: #34748.