Add missing error event listener key to the FakeXMLHttpRequest base c…#1042
Closed
overcaffeinated wants to merge 1 commit intosinonjs:masterfrom
Closed
Add missing error event listener key to the FakeXMLHttpRequest base c…#1042overcaffeinated wants to merge 1 commit intosinonjs:masterfrom
overcaffeinated wants to merge 1 commit intosinonjs:masterfrom
Conversation
…lass. Without this, the changes in commit 2cfbacd cause an onerror function handler on the FakeXMLHttpRequest instance to never be called, since the "error" event is triggered but there is no event listener listening for it. This results in regular error handlers not being called for legitimate error responses (like 500 Internal Server Error).
Contributor
|
Thanks for you contribution, but @fearphage beat you too it with #1041 :-) FYI, I would have needed a test that asserted that the new behavior worked. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose (TL;DR) - mandatory
Add missing "error" event listener key to FakeXmlHttpRequest, which is now not being called as a result of the change referenced in issue #1040
Background (Problem in detail) - optional
A recent change ( 2cfbacd ) that was merged and pushed in 1.17.4 fixed a previous regression (onerror incorrectly being called when an XmlHttpRequest is aborted by the user) but caused the legitimate error handler for XmlHttpRequests to not be called. This happens because the readyStateChange function was changed to trigger an "error" ProgressEvent instead of relying on the FakeXmlHttpRequest.abort function calling onerror directly (this was incorrect behavior).
The problem is that FakeXmlHttpRequest does not currently have a listener for the newly thrown error event, and thus fails to call error handlers attached to the request when an error happens. This change adds the missing listener key to eventListeners and ensures that xhr.onerror is called if it's defined and is a function.
There is ongoing discussion for this in #1040
How to verify - mandatory
npm install-- I have verified this change with my tests in a private repository. I don't have a clean simple example to provide currently, but can provide one if needed.
Without this, the changes in commit 2cfbacd cause
an onerror function handler on the FakeXMLHttpRequest instance to never be called,
since the "error" event is triggered but there is no event listener listening for
it. This results in regular error handlers not being called for legitimate error
responses (like 500 Internal Server Error).