TransferManager uploadFileInChunks does not error when aborted

[tcrognon]

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

• Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues
• Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js
• Check our Troubleshooting guide: https://github.com/googleapis/google-cloud-node/blob/main/docs/troubleshooting.md
• Check our FAQ: https://github.com/googleapis/google-cloud-node/blob/main/docs/faq.md
• Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md
• Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs
• Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples
• 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;:

google-cloud-node/handwritten/storage/src/transfer-manager.ts

Line 955 in b5b5afb

return;

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.