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

storage: handle errors in read stream #746

Conversation

stephenplusplus
Copy link
Contributor

Fixes #724
Fixes #745 (comment)

@beshkenadze caught a bug in how we were handling responses from a file.createReadStream:

If not authorized request (403 error), then there is a exception. ;)

/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/lib/storage/file.js:552
        res.headers['x-goog-hash'].split(',').forEach(function(hash) {
                                  ^
TypeError: Cannot read property 'split' of undefined
    at DestroyableTransform.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/lib/storage/file.js:552:35)
    at DestroyableTransform.emit (events.js:107:17)
    at DestroyableTransform.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/index.js:31:26)
    at DestroyableTransform.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/node_modules/on-everything/index.js:12:17)
    at DestroyableTransform.obj.(anonymous function) [as emit] (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/node_modules/stubs/index.js:28:10)
    at StreamCache.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/index.js:31:26)
    at StreamCache.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/node_modules/on-everything/index.js:12:17)
    at StreamCache.obj.(anonymous function) [as emit] (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/node_modules/stubs/index.js:28:10)
    at Request.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/index.js:31:26)
    at Request.<anonymous> (/Users/akira/Projects/My/ReviewHub/src/node_modules/gcloud/node_modules/stream-forward/node_modules/on-everything/index.js:12:17)

@beshkenadze - #745 (comment)

In a recent commit, I mistakenly thought we could safely remove util.handleResp from the request stream's complete event. That was wrong, but thankfully, we're still on master without having made a release yet!

This adds that back in, so that errors are properly handled, as well as tests to cover that case.

And while I was at it, I fixed up #724, which exposes the abort function from retry-request. We use this to manually abort the request stream in the event of an error.

@stephenplusplus stephenplusplus added the api: storage Issues related to the Cloud Storage API. label Jul 26, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 26, 2015
callmehiphop added a commit that referenced this pull request Jul 27, 2015
…response-handling

storage: handle errors in read stream
@callmehiphop callmehiphop merged commit 7efd2a8 into googleapis:master Jul 27, 2015
@callmehiphop
Copy link
Contributor

Thanks!

sofisl pushed a commit that referenced this pull request Jan 10, 2023
- [ ] Regenerate this pull request now.

PiperOrigin-RevId: 474338479

Source-Link: googleapis/googleapis@d5d35e0

Source-Link: googleapis/googleapis-gen@efcd3f9
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZWZjZDNmOTM5NjJhMTAzZjY4ZjAwM2UyYTFlZWNkZTZmYTIxNmEyNyJ9
sofisl pushed a commit that referenced this pull request Jan 10, 2023
🤖 I have created a release *beep* *boop*
---


## [4.2.0](googleapis/nodejs-dlp@v4.1.1...v4.2.0) (2022-09-22)


### Features

* Add Deidentify action ([#742](googleapis/nodejs-dlp#742)) ([27bb912](googleapis/nodejs-dlp@27bb912))


### Bug Fixes

* Do not import the whole google-gax from proto JS ([#1553](https://github.com/googleapis/nodejs-dlp/issues/1553)) ([#741](googleapis/nodejs-dlp#741)) ([655d6af](googleapis/nodejs-dlp@655d6af))
* Preserve default values in x-goog-request-params header ([#746](googleapis/nodejs-dlp#746)) ([7c53b9f](googleapis/nodejs-dlp@7c53b9f))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
sofisl pushed a commit that referenced this pull request Jan 24, 2023
* fix: use require() to load JSON protos

The library is regenerated with gapic-generator-typescript v1.3.1.

Committer: @alexander-fenster
PiperOrigin-RevId: 372468161

Source-Link: googleapis/googleapis@75880c3

Source-Link: googleapis/googleapis-gen@77b1804

* 🦉 Updates from OwlBot

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Jeff Ching <chingor@google.com>
sofisl pushed a commit that referenced this pull request Jan 25, 2023
* fix: use require() to load JSON protos

The library is regenerated with gapic-generator-typescript v1.3.1.

Committer: @alexander-fenster
PiperOrigin-RevId: 372468161

Source-Link: googleapis/googleapis@75880c3

Source-Link: googleapis/googleapis-gen@77b1804

* 🦉 Updates from OwlBot

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Jeff Ching <chingor@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage- error req.abort is not a function
3 participants