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.

Sign up for GitHub

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

Jump to bottom

Storage: BlobWriteChannel generates NullPointerException in some failure cases (e.g. networking issues) #839

Closed
AdamBSteele opened this issue May 19, 2021 · 2 comments · Fixed by #846
Closed

Storage: BlobWriteChannel generates NullPointerException in some failure cases (e.g. networking issues) #839

AdamBSteele opened this issue May 19, 2021 · 2 comments · Fixed by #846
Assignees
@BenWhitehead
Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. status: investigating The issue is under investigation, which is determined to be non-trivial. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@AdamBSteele
Copy link

Problem

BlobWriteChannel generates NullPointerException in some failure cases (e.g. due to a networking error).
Specifically, BlobWriteChannel:176 might retrieve null StorageObject which causes BlobWriteChannel:180 to generate NullPointerException when trying to dereference it.

This unhandled exception makes it difficult to clearly resolve issues and resume any impacted operations/scripts. As an engineer/on call, I want logs to be more clear (unnecessary exceptions not to be thrown at all if that's possible), so I can get issues resolved faster with less confusion and spend less time on manual workarounds.

Environment details

  1. Specify the API at the beginning of the title. For example, "BigQuery: ...").
    General, Core, and Other are also allowed as types
  2. OS type and version:
  3. Java version:
  4. storage version(s): 1.113.14 and 1.114.0.

Steps to reproduce

  1. Simulate faulty network (dropped packets ~35%, less works too, but takes more time to catch)
  2. Call BlobWriteChannel
  3. See NullPointerException

Stack trace

java.lang.RuntimeException: java.util.concurrent.ExecutionException: ... omitted ...
Caused by: ...: Unable to write ...
... omitted ...
Caused by: java.lang.RuntimeException: Unable to write ... to gs://...
... omitted ...
... 10 more
Caused by: com.google.cloud.storage.StorageException: java.lang.NullPointerException
at com.google.cloud.storage.StorageException.translateAndThrow(StorageException.java:81)
at com.google.cloud.storage.BlobWriteChannel.flushBuffer(BlobWriteChannel.java:212)
at com.google.cloud.BaseWriteChannel.close(BaseWriteChannel.java:151)
at java.base/java.nio.channels.Channels$1.close(Channels.java:177)
at java.base/java.io.FilterOutputStream.close(FilterOutputStream.java:188)
at java.base/java.io.FilterOutputStream.close(FilterOutputStream.java:188)
at org.apache.avro.file.DataFileWriter.close(DataFileWriter.java:461)
... omitted ...
... 14 more
Caused by: java.lang.NullPointerException
at com.google.cloud.storage.BlobWriteChannel$1.run(BlobWriteChannel.java:180)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
at com.google.api.gax.retrying.DirectRetryingExecutor.submit(DirectRetryingExecutor.java:105)
at com.google.cloud.RetryHelper.run(RetryHelper.java:76)
at com.google.cloud.RetryHelper.runWithRetries(RetryHelper.java:50)
at com.google.cloud.storage.BlobWriteChannel.flushBuffer(BlobWriteChannel.java:157)
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage API. label May 19, 2021
@BenWhitehead BenWhitehead added priority: p2 Moderately-important priority. Fix may not be included in next release. status: investigating The issue is under investigation, which is determined to be non-trivial. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels May 20, 2021
@BenWhitehead BenWhitehead mentioned this issue May 26, 2021
Merged
@BenWhitehead BenWhitehead self-assigned this Jun 1, 2021
BenWhitehead added a commit that referenced this issue Jun 1, 2021
@BenWhitehead
fix: improve error detection and reporting for BlobWriteChannel retry…
d0f2184 
… state (#846)

Add new checks to ensure a more informative error than NullPointerException is thrown if the StorageObject or it's size are unable to be resolved on the last chunk.

Fixes #839
@frankyn frankyn reopened this Jun 1, 2021
@frankyn
Copy link
Member

frankyn commented Jun 1, 2021

This is not yet resolved, IIUC, and the change merged is to provide more debugging context. @BenWhitehead is this accurate?

gcf-merge-on-green bot pushed a commit that referenced this issue Jun 1, 2021
@release-please
chore: release 1.115.0 (#834)
0018bcb 
🤖 I have created a release \*beep\* \*boop\*
---
## [1.115.0](https://www.github.com/googleapis/java-storage/compare/v1.114.0...v1.115.0) (2021-06-01)


### Features

* add `gcf-owl-bot[bot]` to `ignoreAuthors` ([#837](https://www.github.com/googleapis/java-storage/issues/837)) ([fe8e98a](https://www.github.com/googleapis/java-storage/commit/fe8e98a229f472c1f29d206d937690660bfa1444))


### Bug Fixes

* improve error detection and reporting for BlobWriteChannel retry state ([#846](https://www.github.com/googleapis/java-storage/issues/846)) ([d0f2184](https://www.github.com/googleapis/java-storage/commit/d0f2184f4dd2d99a4315f260f35421358d14a2df)), closes [#839](https://www.github.com/googleapis/java-storage/issues/839)
* update BucketInfo translation code to properly handle lifecycle rules ([#852](https://www.github.com/googleapis/java-storage/issues/852)) ([3b1df1d](https://www.github.com/googleapis/java-storage/commit/3b1df1d00a459b134103bc8738f0294188502a37)), closes [#850](https://www.github.com/googleapis/java-storage/issues/850)


### Dependencies

* update dependency com.google.cloud:google-cloud-shared-dependencies to v1.2.0 ([#836](https://www.github.com/googleapis/java-storage/issues/836)) ([c1752ce](https://www.github.com/googleapis/java-storage/commit/c1752ce17d5d723d0ea36c41d98ae2bc9201fec2))
* update kms.version to v0.88.4 ([#830](https://www.github.com/googleapis/java-storage/issues/830)) ([7e3dc28](https://www.github.com/googleapis/java-storage/commit/7e3dc287e4285a9312393179671a78c569e7e869))
* update kms.version to v0.89.0 ([#855](https://www.github.com/googleapis/java-storage/issues/855)) ([29236e9](https://www.github.com/googleapis/java-storage/commit/29236e9d2eefb0e64b1b9bbfc532f4c3ae3e9ea4))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@BenWhitehead BenWhitehead mentioned this issue Jul 9, 2021
Merged
BenWhitehead added a commit that referenced this issue Jul 13, 2021
@BenWhitehead
fix: correct lastChunk retry logic in BlobWriteChannel (#918)
ab0228c 
Add new method StorageRpc#queryResumableUpload which allows getting a shallow StorageObject for a resumable upload session which is complete.

Update BlobWriteChannel to use StoageRpc#queryResumableUpload instead of StorageRpc#get when attempting to validate the upload size of an object when it determines the upload is complete and is on the last chunk.

If a BlobWriteChannel is opened with a conditional like IfGenerationMatch it is not possible to simply get the object, as the object can drift generationally while the resumable upload is being performed.

Related to #839
@BenWhitehead
Copy link
Collaborator

Received confirmation from the customer they're no longer seeing the error after upgrading to the latest version.

BenWhitehead added a commit that referenced this issue Sep 28, 2021
@BenWhitehead @JesseLovelace @tritone
fix(1.113.14-sp): backport critical bug fixes (#993)
447d35d 
* feat: Remove client side vaildation for lifecycle conditions (#816)

* Remove client side vaildation for lifecycle conditions

* fix lint and suggest updating

(cherry picked from commit 5ec84cc)

* fix: update BucketInfo translation code to properly handle lifecycle rules (#852)

Fixes #850

(cherry picked from commit 3b1df1d)

* fix: improve error detection and reporting for BlobWriteChannel retry state (#846)

Add new checks to ensure a more informative error than NullPointerException is thrown if the StorageObject or it's size are unable to be resolved on the last chunk.

Fixes #839

(cherry picked from commit d0f2184)

* fix: correct lastChunk retry logic in BlobWriteChannel (#918)

Add new method StorageRpc#queryResumableUpload which allows getting a shallow StorageObject for a resumable upload session which is complete.

Update BlobWriteChannel to use StoageRpc#queryResumableUpload instead of StorageRpc#get when attempting to validate the upload size of an object when it determines the upload is complete and is on the last chunk.

If a BlobWriteChannel is opened with a conditional like IfGenerationMatch it is not possible to simply get the object, as the object can drift generationally while the resumable upload is being performed.

Related to #839

(cherry picked from commit ab0228c)

* test: remove error string matching (#861)

It looks like the text for this error on the backend has changed
(sometimes) from "Precondition Failed" to "At least one of the
pre-conditions you specified did not hold". I don't think it's
really necessary to check the exact message in any case given
that we do check for a code of 412, which implies a precondition
failure. I added a check of the error Reason instead,  which is more
standardized.

Fixes #853

(cherry picked from commit 146a3d3)

Co-authored-by: JesseLovelace <43148100+JesseLovelace@users.noreply.github.com>
Co-authored-by: Chris Cotter <cjcotter@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BenWhitehead BenWhitehead

Labels
api: storage Issues related to the googleapis/java-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. status: investigating The issue is under investigation, which is determined to be non-trivial. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: improve error detection and reporting for BlobWriteChannel retry state
3 participants
@BenWhitehead @frankyn @AdamBSteele