Skip to content

Extern xhr error property#319

Merged
joeyparrish merged 1 commit intoshaka-project:v1.6.xfrom
wader:extern-error-xhr-property
Apr 5, 2016
Merged

Extern xhr error property#319
joeyparrish merged 1 commit intoshaka-project:v1.6.xfrom
wader:extern-error-xhr-property

Conversation

@wader
Copy link

@wader wader commented Apr 1, 2016

Make it possible to access the response when requests fails.

I tried to change to ['xhr'] = but then the tests failed because of type error

Hope it's correct to target v1.6.x branch.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@wader
Copy link
Author

wader commented Apr 1, 2016

Related to #317

@joeyparrish
Copy link
Member

Please read CONTRIBUTING.md and follow the instructions there.

@wader
Copy link
Author

wader commented Apr 4, 2016

I signed it!

On Fri, Apr 1, 2016 at 5:01 PM, googlebot notifications@github.com wrote:

Thanks for your pull request. It looks like this may be your first
contribution to a Google open source project. Before we can look at your
pull request, you'll need to sign a Contributor License Agreement (CLA).

Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify.
Thanks.


If you've already signed a CLA, it's possible we don't have your GitHub
username or you're using a different email address. Check your existing CLA
data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's
name.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@googlebot
Copy link

CLAs look good, thanks!

@wader wader force-pushed the extern-error-xhr-property branch 2 times, most recently from 017646a to 4a881b9 Compare April 4, 2016 12:42
@wader
Copy link
Author

wader commented Apr 4, 2016

@joeyparrish CLA signed and test added. But the test is kind of useless as currently unit tests only run on uncompiled source... i tried to run it on compiled source but then a lot of other unrelated tests fails 😞

@joeyparrish
Copy link
Member

Yeah, you're right. The new test doesn't really help anything in this case. We could go ahead and drop it, IMO.

@joeyparrish joeyparrish self-assigned this Apr 4, 2016
@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Apr 4, 2016
Make it possible to access response when a request fail

shaka-project#317
@wader wader force-pushed the extern-error-xhr-property branch from 4a881b9 to 464384a Compare April 5, 2016 07:52
@wader
Copy link
Author

wader commented Apr 5, 2016

@joeyparrish Ok, spec file change is gone now

@joeyparrish joeyparrish merged commit e244a1a into shaka-project:v1.6.x Apr 5, 2016
@joeyparrish
Copy link
Member

Thanks!

@wader
Copy link
Author

wader commented Apr 6, 2016

@joeyparrish would it be possible to get a new 1.6.x package version to npm?

@wader
Copy link
Author

wader commented Apr 8, 2016

Im looking at the 2.0 code to see how we could access the response. My guess is that in lib/net/http_plugin.js the xhr instance needs to be assign to the error somehow or?

@joeyparrish
Copy link
Member

@wader, sure I can push a new v1.6.x release with bug fixes.

As for 2.0, I'd rather take another approach. You need the body of the HTTP response, correct? What if instead of passing up XHR, we attached the response text to error.data instead?

@joeyparrish
Copy link
Member

v1.6.5 is up on npm now. Thanks!

@wader
Copy link
Author

wader commented Apr 9, 2016

@joeyparrish Great!
Both body and status code would be nice

shaka-bot pushed a commit that referenced this pull request Apr 12, 2016
Requested in #319

Change-Id: I2f52519d7a45769a0d2db3c038c9c04cf7fb59c1
@joeyparrish
Copy link
Member

@wader, v2 now includes the response body in the error object. Thanks!

@wader
Copy link
Author

wader commented Apr 13, 2016

@joeyparrish Thanks! and now i see that status code was already there, nice

@wader wader deleted the extern-error-xhr-property branch April 13, 2016 07:46
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

status: archived Archived and locked; will not be updated type: bug Something isn't working correctly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants