Skip to content

Allow making a POST request with an ArrayBuffer body #227

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

Merged
merged 1 commit into from
Nov 2, 2015

Conversation

joshuawarner32
Copy link

This enables POSTing with an ArrayBuffer body, but doesn't change the existing support for receiving ArrayBuffer responses (via blob)

@joshuawarner32
Copy link
Author

The travis tests are failing with this error, which I'm not able to reproduce locally, but I don't think it relates to my changes. Any tips are appreciated.

./script/test
....https://saucelabs.com/jobs/f9fc0d513cc9426690b8a03b710a27b2
["Windows 7", "internet explorer", "9"]
"Invalid tunnel_identifier 402.5"
./script/saucelabs-result:13:in `<main>': undefined method `+' for nil:NilClass (NoMethodError)
make: *** [test] Error 1

@@ -363,7 +367,7 @@
}

if ('responseType' in xhr && support.blob) {
xhr.responseType = 'blob'
xhr.responseType = 'arrayBuffer'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why was this particular change needed?

Copy link
Author

Choose a reason for hiding this comment

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

Whoops! Good catch. That was some code I had in to debug what turned out to be an unrelated issue. I've since removed this change.

@mislav
Copy link
Contributor

mislav commented Oct 30, 2015

I'm sorry for the CI failure, it wasn't your fault. I just updated master with an attempted fix for CI. Can you merge master into your branch and push to update this PR? It should get the CI passing. Thanks

@joshuawarner32
Copy link
Author

Ok, I rebased onto master, but it looks like one of the jobs is still failing. Is there a way I can see which tests are failing? I'm not seeing it in the build output on travis.

@mislav
Copy link
Contributor

mislav commented Oct 31, 2015

It failed for IE 9.

You'll probably have to skip your added test on browsers that don't support ArrayBuffer.

@joshuawarner32
Copy link
Author

@mislav - Looks like the tests passed this time. Are there any other concerns you have?

mislav added a commit that referenced this pull request Nov 2, 2015
Allow making a POST request with an ArrayBuffer body
@mislav mislav merged commit 4db9f7b into JakeChampion:master Nov 2, 2015
@mislav
Copy link
Contributor

mislav commented Nov 2, 2015

Thanks!

@joshuawarner32
Copy link
Author

Would it be possible to get a new npm release with this code? If now's not the right time, that's fine - we can use a full git url for the time being.

@mislav
Copy link
Contributor

mislav commented Nov 2, 2015

I've pushed a tag right now. Should appear on npm automatically.

@joshuawarner32
Copy link
Author

Hmm, I think something may have gone wrong with that automated release process. I don't see the new release on npm yet. Or, perhaps, it's just very slow; feel free to say "be patient!".

@mislav
Copy link
Contributor

mislav commented Nov 3, 2015

Hm something went wrong with auto-publishing to npm from Travis CI. I published manually now.

@joshuawarner32
Copy link
Author

Looks good, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 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.

2 participants