TransferManager uploadFileInChunks does not error when aborted

Open Beginner friendly
#9,193 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

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

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.

Description

Please make sure you have searched for information in the following guides.
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:

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.

Dominant language
TypeScript
Stars
3.2k
Forks
714
Avg merge
2d 13h
Merged PRs (30d)
106

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from googleapis/google-cloud-node

All issues in googleapis/google-cloud-node

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.