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

Check for client.abort function before executing. #1651

Closed
wants to merge 1 commit into from

Conversation

graemeworthy
Copy link

line is now identical to the open() method, on #435 in the same file.

line is now identical to the open() method, on jsdom#435 in the same file.
@domenic
Copy link
Member

domenic commented Nov 10, 2016

Can you provide a test case that fails when this check is not present?

@domenic
Copy link
Member

domenic commented Dec 18, 2016

Closing due to lack of response, but happy to reopen and merge if we can add a test for this.

@domenic domenic closed this Dec 18, 2016
@vincemtnz
Copy link

This blows up for me in a test (Jest) with an url variable that was undefined (so: $.get(undefined).then(...)), but the same goes for any other falsy url values.

  ● Test suite failed to run
    TypeError: client.abort is not a function

      at XMLHttpRequest.abort (node_modules/jsdom/lib/jsdom/living/xmlhttprequest.js:369:16)
      at Object.abort (node_modules/jsdom/lib/jsdom/living/xhr-utils.js:315:13)
      at RequestManager.close (node_modules/jsdom/lib/jsdom/living/nodes/Document-impl.js:146:21)
      at Window.close (node_modules/jsdom/lib/jsdom/browser/Window.js:362:29)
          at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:169:7)

Firing up the debugger shows that client is an EventEmitter but does not have an abort fn. I did notice an abort fn under _events though.

screen shot 2017-06-22 at 20 43 12

@domenic
Copy link
Member

domenic commented Jun 22, 2017

Please open a new issue following the issue template; in particular pay attention to the instructions about a minimal example, not involving other libraries like Jest.

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.

None yet

3 participants