googleapis / googleapis/google-cloud-node

TransferManager uploadFileInChunks does not error when aborted

Open Beginner friendly
#9,193 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
3.2k
Forks
712
Avg merge
2d 3h
Merged PRs (30d)
99

Description

### Please make sure you have searched for information in the following guides.

- [x] Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues
- [x] Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js
- [x] Check our Troubleshooting guide: https://github.com/googleapis/google-cloud-node/blob/main/docs/troubleshooting.md
- [x] Check our FAQ: https://github.com/googleapis/google-cloud-node/blob/main/docs/faq.md
- [x] Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md
- [x] Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs
- [x] Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples
- [x] Check the API's issue tracker: https://cloud.google.com/support/docs/issue-trackers

### Library Name

@google-cloud/storage

### A screenshot that you have tested with "Try this API".

N/A

### Link to the code that reproduces this issue. A link to a **public** Github Repository or gist with a minimal reproduction.

https://gist.github.com/tcrognon/3102b05ce3e0aa902e22decb5be0a788

### A step-by-step description of how to reproduce the issue, based on the linked reproduction.

1. Replace SOME_BUCKET with your GCP test bucket.
2. Replace SOME_FILE_PATH with a file to upload to SOME_BUCKET.
3. Run the code and somehow cause one of the PUT requests to fail, but don't just disable internet because the DELETE request (to abort the XML multipart upload) needs to succeed.

### A clear and concise description of what the bug is, and what you expected to happen.

`uploadFileInChunks` performs an XML multipart upload. This is:
1) A POST to create the multipart upload.
2) A series of PUTs to upload the parts.
3) A POST to complete the multipart upload.
4) A DELETE to abort the multipart upload if 2) or 3) fails.

If 4) happens and is successful, `uploadFileInChunks` returns `undefined`. I expected it would throw. All other error throw.

Here is the offending `return;`: https://github.com/googleapis/google-cloud-node/blob/b5b5afb9e053744ff7c3f6334aa2a60f86466c84/handwritten/storage/src/transfer-manager.ts#L955

### A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. **

Returning `undefined` in this single case is very unexpected and the documentation heavily implies that if `uploadFileInChunks` doesn't throw, then the file was written successfully:
- https://docs.cloud.google.com/nodejs/docs/reference/storage/latest/storage/transfermanager#_google_cloud_storage_TransferManager_uploadFileInChunks_member_1_
- https://docs.cloud.google.com/storage/docs/multipart-uploads#storage-upload-object-chunks-nodejs

If auth problem, throws. If invalid bucket or file, throws. If initial POST fails, throws. If subsequent PUTs/POST fails and the abort fails, throws. But if the abort succeeds, returns undefined.

I experienced silent data loss due to this. Code continued after aborted uploads thinking they were successfully written.

Contributor guide

Open the contributing guide

Research direction

Start in handwritten/storage/src/transfer-manager.ts at the linked return in uploadFileInChunks, then review the reproduction gist to understand the failed part upload and successful abort. Confirm that an upload failure still propagates an error after the multipart upload is aborted, rather than returning undefined.

Written by the indexing model from the issue text.

Assessment

Tech stack
google-cloud, node.js, typescript
Domain
backend, cloud
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.