Skip to content

Conversation

@jorgee
Copy link
Contributor

@jorgee jorgee commented Jan 9, 2026

Alternative for #6669

This pull request refactors error handling for AWS S3 operations throughout the Nextflow AWS NIO plugin. The main improvement is the consistent conversion of AWS SDK exceptions into standard Java IO exceptions, which simplifies error management and improves reliability. Several method signatures now declare throws IOException, and exception handling logic has been centralized in the S3Client class. Related test cases have also been updated to reflect these changes.

Error handling improvements:

  • Centralized AWS SDK exception conversion in S3Client: All S3 operations now catch SdkException and convert them to appropriate IO exceptions (NoSuchFileException, AccessDeniedException, etc.) via the new convertAwsException method. Method signatures have been updated to throw IOException.

  • Updated dependent classes (S3FileSystem, S3Iterator, S3ObjectSummaryLookup) to handle IOException and wrap where necessary with UncheckedIOException. This ensures that errors propagate correctly and are handled uniformly.

  • Refactored getAccessControl and lookup methods to throw IOException and handle both bucket and object ACL retrieval more robustly. [1] [2]

Test updates:

  • Re-enabled and updated previously ignored tests in AwsS3NioTest.groovy to align with new error handling, ensuring correct exceptions are thrown and caught.

Code cleanup:

  • Removed redundant AWS exception handling code from S3FileSystemProvider and related classes, relying instead on the centralized logic in S3Client. [1] [2] [3]

  • Removed unused imports and dead code related to old exception handling. [1] [2] [3]

Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@netlify
Copy link

netlify bot commented Jan 9, 2026

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 41986dc
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/6960fdcc28eb5a000898ce93

@jorgee
Copy link
Contributor Author

jorgee commented Jan 9, 2026

@ben, looking at the places where we can get the error Unable to marshall request to JSON: Key cannot be empty. It can happen when Files.createDirectory(path) and Files.delete(path) when path is a bucket root (s3://bucke-name).
In the AwsS3NioTest, the tests to create and delete buckets are ignored. Should we support them or are they intentionally unsupported? The fix is easy just checking if key is key is empty and run createBucket or deleteBucket.

@bentsherman
Copy link
Member

My gut feeling is that creating / deleting buckets is outside the scope of our S3 integration. It would be like installing or removing a network drive in an HPC... doesn't seem like the kind of thing that should be done by a pipeline.

But if you feel the error message can be improved that's fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants