Skip to content

Fix NIO contract violations in Seqera filesystem#7037

Merged
bentsherman merged 1 commit into260310-seqera-dataset-fsfrom
fix/seqera-fs-review-fixes
Apr 16, 2026
Merged

Fix NIO contract violations in Seqera filesystem#7037
bentsherman merged 1 commit into260310-seqera-dataset-fsfrom
fix/seqera-fs-review-fixes

Conversation

@pditommaso
Copy link
Copy Markdown
Member

Summary

Fixes 6 issues found during code review of #6946:

  • resolveSibling NPE: SeqeraPath.resolveSibling() called getParent().resolve() without null-checking. NIO contract says return other when parent is null.
  • read-after-close: DatasetInputStream.read() did not check the open flag. Now throws ClosedChannelException.
  • raw Exception: TowerClient.sendStreamingRequest() threw raw Exception for unknown HTTP status codes. Changed to IOException.
  • getUserId NPE: SeqeraDatasetClient.getUserId() did getUserInfo()?.id as long which NPEs when info or id is null. Added explicit null guard.
  • filter IOException swallowed: newDirectoryStream caught filter IOException and returned false. NIO spec requires throwing DirectoryIteratorException. Fixed.
  • ambiguous checkResponse: Renamed SeqeraDatasetClient.checkResponse to checkFsResponse to disambiguate from TowerClient.checkResponse.

Test plan

  • ./gradlew :plugins:nf-tower:test -- all tests pass

- Fix resolveSibling NPE when called on root/shallow paths (NIO contract:
  return `other` when parent is null)
- Guard DatasetInputStream.read() against reads after close() by checking
  the `open` flag and throwing ClosedChannelException
- Use IOException instead of raw Exception in sendStreamingRequest for
  unknown HTTP status codes
- Add null guard in SeqeraDatasetClient.getUserId() to prevent NPE when
  getUserInfo() returns null
- Propagate DirectoryIteratorException from newDirectoryStream filter
  instead of silently swallowing IOExceptions (per NIO spec)
- Rename checkResponse to checkFsResponse in SeqeraDatasetClient to
  disambiguate from TowerClient.checkResponse

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso pditommaso requested a review from jorgee April 16, 2026 08:51
@bentsherman bentsherman merged commit da8bad3 into 260310-seqera-dataset-fs Apr 16, 2026
21 checks passed
@bentsherman bentsherman deleted the fix/seqera-fs-review-fixes branch April 16, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants