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

fix: update UploadCallable to use createFrom to avoid NPE trying to resolve resulting object #2086

Merged
merged 5 commits into from
Jun 23, 2023

Conversation

sydney-munro
Copy link
Collaborator

@sydney-munro sydney-munro commented Jun 21, 2023

This PR has a few parts to it, all small enough to where I did not feel the need to separate.

Major Addition

There was a bug where the storageObject was being accessed before the WriteChannel closes, when the upload is not finished WriteChannel returns a null storageObject resulting in an NPE when we go to decode the storageObject. see BlobWriteChannel#storageObject for where this is documented.

First Pass (up to commit 76462df): addressed this by trying to process the storageObject outside of the scope of the try block, which would ensure the write has completed. All we have to do is manually ensure that the channel is closed.

Refactor (commit 976b9ba): utilizes createFrom, this addition is to both relieve the upload call of handling the Write/FileChannels directly and also allows us to take advantage of any performance improvements being made. The drawback of this is we will regress on our TransferStatus reporting as we no longer have direct byte access to track upload progress. The plan is to short term handle this via Status code checks and long term add in progress monitoring.

Minor Additions

  • Add additional assertions to tests to prevent a bug like this from lurking about.
  • Add the skipIfExists functionality

@sydney-munro sydney-munro requested a review from a team as a code owner June 21, 2023 23:19
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/java-storage API. labels Jun 21, 2023
@BenWhitehead BenWhitehead changed the title fix: Fix bug where the StorageObject is accessed before WriteChannel closes fix: update UploadCallable to use createFrom to avoid NPE trying to resolve resulting object Jun 23, 2023
@sydney-munro sydney-munro merged commit 6769a2b into feat/transfer-manager Jun 23, 2023
15 checks passed
@sydney-munro sydney-munro deleted the preconditions branch June 23, 2023 16:58
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 googleapis/java-storage API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants