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

Google Storage NIO: FileAlreadyExistsException exception is not honoured when overwriting files #762

Closed
pditommaso opened this issue Nov 19, 2021 · 7 comments · Fixed by #815
Assignees
Labels
api: storage Issues related to the googleapis/java-storage-nio API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@pditommaso
Copy link

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

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: any
  3. Java version: Java 8 and later
  4. storage_nio version(s): com.google.cloud:google-cloud-nio:0.122.0 and later

Steps to reproduce

  1. Create a file foo.txt into my-bucket
  2. Create a file bar.txt into my-bucket
  3. Copy foo.txt to bar.txt

Code example

        given:
        def foo = CloudStorageFileSystem.forBucket('nf-bucket').getPath('foo.txt')
        def bar = CloudStorageFileSystem.forBucket('nf-bucket').getPath('bar.txt')
        and:
        Files.createFile(foo)
        Files.createFile(bar)
        when:
        Files.copy(foo, bar)
        then:
        thrown(FileAlreadyExistsException)

Stack trace

com.google.cloud.storage.StorageException: 412 Precondition Failed
PUT https://storage.googleapis.com/upload/storage/v1/b/nf-bucket/o?uploadType=resumable&name=foo.txt&ifGenerationMatch=0&upload_id=ADPycdtER9Udv5f3mdmhElr4rbAf-0U0L59G6pHmMw92d6SUtppicKL0slMnCdx8JhX1_2MPZo9MBUph1Q9NqQXuUjIUVoCeHQ
{
  "error": {
    "code": 412,
    "message": "At least one of the pre-conditions you specified did not hold.",
    "errors": [
      {
        "message": "At least one of the pre-conditions you specified did not hold.",
        "domain": "global",
        "reason": "conditionNotMet",
        "locationType": "header",
        "location": "If-Match"
      }
    ]
  }
}


	at com.google.cloud.storage.spi.v1.HttpStorageRpc.translate(HttpStorageRpc.java:231)
	at com.google.cloud.storage.spi.v1.HttpStorageRpc.writeWithResponse(HttpStorageRpc.java:822)
	at com.google.cloud.storage.BlobWriteChannel$1.run(BlobWriteChannel.java:69)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	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:61)
	at com.google.cloud.BaseWriteChannel.close(BaseWriteChannel.java:151)
	at com.google.cloud.storage.contrib.nio.CloudStorageWriteChannel.close(CloudStorageWriteChannel.java:55)
	at java.nio.file.Files.createFile(Files.java:632)

External references such as API reference guides

Any additional information below

This work as expected up to version 0.121.2

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/java-storage-nio API. label Nov 19, 2021
@shaffeeullah shaffeeullah added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Nov 19, 2021
pditommaso added a commit to nextflow-io/nextflow that referenced this issue Nov 19, 2021
Latest version google-cloud-nio library introduced an issue
when the publishDir directive tries to overwrite a file already
existing in the target directory. An Google custom exception
is thrown instead of the expected FileAlreadyExistsException error.

This commit reverts the dependecy to the last know version having
the expected behaviour.

See googleapis/java-storage-nio#762

Fixes #2455
pditommaso added a commit to nextflow-io/nextflow that referenced this issue Nov 19, 2021
Latest version google-cloud-nio library introduced an issue
when the publishDir directive tries to overwrite a file already
existing in the target directory. An Google custom exception
is thrown instead of the expected FileAlreadyExistsException error.

This commit reverts the dependecy to the last know version having
the expected behaviour.

See googleapis/java-storage-nio#762

Fixes #2455
pditommaso added a commit to nextflow-io/nextflow that referenced this issue Nov 20, 2021
Latest version google-cloud-nio library introduced an issue
when the publishDir directive tries to overwrite a file already
existing in the target directory. An Google custom exception
is thrown instead of the expected FileAlreadyExistsException error.

This commit reverts the dependecy to the last know version having
the expected behaviour.

See googleapis/java-storage-nio#762

Fixes #2455
pditommaso added a commit to nextflow-io/nextflow that referenced this issue Nov 22, 2021
Latest version google-cloud-nio library introduced an issue
when the publishDir directive tries to overwrite a file already
existing in the target directory. An Google custom exception
is thrown instead of the expected FileAlreadyExistsException error.

This commit reverts the dependecy to the last know version having
the expected behaviour.

See googleapis/java-storage-nio#762

Fixes #2455
@BenWhitehead
Copy link
Collaborator

Hi @pditommaso,

The error you're receiving is an optimistic copy protection to try and help prevent accidentally overwriting any data.

In this case, since you do in fact want to overwrite you'll need to add an option to your Files.copy call

    Files.copy(foo, bar, java.nio.file.StandardCopyOption.REPLACE_EXISTING);

Adding that option will ensure the request that is sent is tolerant of an overwrite happening.

@BenWhitehead BenWhitehead added type: question Request for information or clarification. Not an issue. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Nov 22, 2021
@pditommaso
Copy link
Author

Hi Ben, thanks for the swift reply. Yes, I've realised that however, it would not make sense to honour the Files.copy contract and throws a FileAlreadyExistsException instead of a custom exception when the file already exists and replace flag is not specified?

@BenWhitehead
Copy link
Collaborator

Thank you for clarifying, I missed that the first time around.

According to the definition of Files.copy you're correct and I think it makes sense. I'm not super familiar with the internal design of the errors and exception for this library. I'll have to check with some folks and see if there is anything that would prevent us from making the change.

@BenWhitehead
Copy link
Collaborator

I was finally able to get consensus from several folks and determined we're okay moving forward adding the special handling. Unfortunately, I can't yet give a specific timeframe for when the fix will be implemented and released.

We'll provide an update when we know more time wise.

@BenWhitehead BenWhitehead added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed type: question Request for information or clarification. Not an issue. labels Dec 8, 2021
phue pushed a commit to phue/nextflow that referenced this issue Jan 25, 2022
Latest version google-cloud-nio library introduced an issue
when the publishDir directive tries to overwrite a file already
existing in the target directory. An Google custom exception
is thrown instead of the expected FileAlreadyExistsException error.

This commit reverts the dependecy to the last know version having
the expected behaviour.

See googleapis/java-storage-nio#762

Fixes nextflow-io#2455
@BenWhitehead
Copy link
Collaborator

Sorry for the long delay on this @pditommaso, the fix has just been merged and will go out in the next release (I would guess next week some time).

@pditommaso
Copy link
Author

Nice, thanks a lot. For the records, is the target release number already know (so I can keep an eye on it)?

@BenWhitehead
Copy link
Collaborator

I expect that it'll release in v 0.123.21. You can follow #816 for release status updates. Our robots will comment and update that issues after it's merged and the artifacts have been pushed to maven central.

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-nio API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
3 participants