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

report() should allow handling of non-200 Stackdriver responses #32

Closed
calbach opened this issue Mar 15, 2018 · 0 comments · Fixed by #59
Closed

report() should allow handling of non-200 Stackdriver responses #32

calbach opened this issue Mar 15, 2018 · 0 comments · Fixed by #59

Comments

@calbach
Copy link

calbach commented Mar 15, 2018

Current API signature:

report(e, callback), where callback happens on both success or failure (passed XHR error on transaction failure).

In the event that the Stackdriver XHR succeeds with a non-200, e.g. 403 due to API key origin restriction, callback is invoked without parameters, so it looks like a success to the client. This is because XMLHttpRequest.onerror is not dependent on the HTTP status code: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequestEventTarget/onerror

Relevant code:

xhr.onerror = function(e) {

Possibly this method could be changed to accept two callbacks, or else use a Promise. The error path should trigger if the HTTP response status is not a 200.

bz2 added a commit to bz2/stackdriver-errors-js that referenced this issue Dec 7, 2018
If the XMLHttpRequest fails or a non-2XX code results the promise
will be rejected with an Error. Otherwise it will resolve with the
message reported included.

Fixes GoogleCloudPlatform#32
bz2 added a commit to bz2/stackdriver-errors-js that referenced this issue Dec 7, 2018
If the XMLHttpRequest fails or a non-2XX code results the promise
will be rejected with an Error. Otherwise it will resolve with the
message reported included.

Fixes GoogleCloudPlatform#32
@steren steren closed this as completed in #59 Dec 9, 2018
steren pushed a commit that referenced this issue Dec 9, 2018
If the XMLHttpRequest fails or a non-2XX code results the promise
will be rejected with an Error. Otherwise it will resolve with the
message reported included.

Fixes #32
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 a pull request may close this issue.

1 participant