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

Test Server #13

Merged
merged 10 commits into from
Oct 16, 2014
Merged

Test Server #13

merged 10 commits into from
Oct 16, 2014

Conversation

josh
Copy link
Contributor

@josh josh commented Oct 15, 2014

WIP

Closes #6.

ok(true)
start()
})
})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to test the network error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I guess we can hit example.com which should fail.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you have another server that listens on a particular port and just closes whatever connection it receives (or sends garbage or something)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O, hmm, I wonder if we can prematurely close the node http res stream and cause the error.

@josh
Copy link
Contributor Author

josh commented Oct 16, 2014

@dgraham think I figured out the test hang and why we saw it on that other project ;) ;). It looks like phantomjs doesn't support Blob though it implements the constructor, its just broken. I haven't looked into a real way to feature detect it yet.

I'm also wondering what the polyfill should do in general if Blobs are unavailable.

@josh
Copy link
Contributor Author

josh commented Oct 16, 2014

CI build https://travis-ci.org/github/fetch/builds/38111889

GH build statuses don't seem to be working.

@dgraham
Copy link
Contributor

dgraham commented Oct 16, 2014

Oh, that makes sense. We've had to polyfill other API for phantomjs because it's so old.

},
'/error': function(res) {
res.destroy();
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aroben this worked well for killing the tcp connection.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@dgraham
Copy link
Contributor

dgraham commented Oct 16, 2014

@dgraham
Copy link
Contributor

dgraham commented Oct 16, 2014

I'm also wondering what the polyfill should do in general if Blobs are unavailable.

I think we have two choices for the blob body promise:

  1. Leave it as is and allow the caller to simply not call response.blob() if their site doesn't support older browsers. Presumably the site has avoided Blob or polyfilled it already.
  2. Reject the promise with a TypeError:
reject(new TypeError('Blob is not supported in this browser'))

@aroben
Copy link

aroben commented Oct 16, 2014

I'd imagine that a browser that implements fetch but doesn't support Blob would leave response.blob (the function itself) undefined. Maybe we can do that?

josh added a commit that referenced this pull request Oct 16, 2014
@josh josh merged commit d85d32d into master Oct 16, 2014
@josh josh deleted the server branch October 16, 2014 17:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test against real server
3 participants