Make an S3 listing the server cuts short an error #333

Open
clawbot wants to merge 1 commits from issue-297-truncated-listing-error into next
Collaborator

Fixes #297.

What was wrong: ListObjects and ListObjectsStream in internal/s3/client.go walk a listing with the SDK paginator. The paginator takes a page marked truncated but carrying no continuation token for the last page and stops without an error. A server answering V2 listings V1-style sends such pages, so List and ListStream returned the first page as the whole listing, and prune treated the blobs of the snapshots on later pages as unreferenced.

What changed: both methods fetch each page through nextListPage, which returns the new ErrNoContinuationToken for such a page. Nothing that calls List or ListStream changed: prune, snapshot list and remote nuke already handle a listing error as a destination that cannot be listed, and prune then deletes nothing.

Test: TestS3ListingCutShortIsAnError points an S3 client at a stub server that answers every request with one such page, and checks that List and ListStream both fail with ErrNoContinuationToken.

Judgement call: the test serves a fixed reply from a stub server rather than wrapping gofakes3 as the issue's throwaway test did, since gofakes3 always sends the token and the wrapper would have to rewrite its replies.

Small extra: the region of the S3 clients in internal/storage/s3_test.go is now a constant, because a third copy of "us-east-1" in the package's tests failed the lint check for repeated strings.

Model: opus-5-5

Fixes https://git.eeqj.de/sneak/vaultik/issues/297. **What was wrong:** `ListObjects` and `ListObjectsStream` in `internal/s3/client.go` walk a listing with the SDK paginator. The paginator takes a page marked truncated but carrying no continuation token for the last page and stops without an error. A server answering V2 listings V1-style sends such pages, so `List` and `ListStream` returned the first page as the whole listing, and `prune` treated the blobs of the snapshots on later pages as unreferenced. **What changed:** both methods fetch each page through `nextListPage`, which returns the new `ErrNoContinuationToken` for such a page. Nothing that calls `List` or `ListStream` changed: `prune`, `snapshot list` and `remote nuke` already handle a listing error as a destination that cannot be listed, and `prune` then deletes nothing. **Test:** `TestS3ListingCutShortIsAnError` points an S3 client at a stub server that answers every request with one such page, and checks that `List` and `ListStream` both fail with `ErrNoContinuationToken`. Judgement call: the test serves a fixed reply from a stub server rather than wrapping gofakes3 as the issue's throwaway test did, since gofakes3 always sends the token and the wrapper would have to rewrite its replies. Small extra: the region of the S3 clients in `internal/storage/s3_test.go` is now a constant, because a third copy of `"us-east-1"` in the package's tests failed the lint check for repeated strings. Model: opus-5-5
clawbot added the needs-review label 2026-10-13 03:39:07 +02:00
clawbot self-assigned this 2026-10-13 03:39:07 +02:00
Author
Collaborator
  • Needs a rebase: the branch conflicts with current next in TODO.md, where the Completed Steps entry for #298 landed at the same spot as this PR's entry. Acceptable: rebase onto current next keeping both entries, this PR's 2026-10-13 entry above the #298 one.

Model: opus-5-5

- Needs a rebase: the branch conflicts with current `next` in `TODO.md`, where the Completed Steps entry for https://git.eeqj.de/sneak/vaultik/issues/298 landed at the same spot as this PR's entry. Acceptable: rebase onto current `next` keeping both entries, this PR's 2026-10-13 entry above the #298 one. Model: opus-5-5
clawbot added needs-rebase and removed needs-review labels 2026-10-13 04:53:50 +02:00
clawbot added 1 commit 2026-10-13 07:09:05 +02:00
The SDK paginator takes a listing page marked truncated but carrying no
continuation token for the last page and stops without an error. A
server answering V2 listings V1-style sends such pages, so ListObjects
and ListObjectsStream returned the first page as the whole listing, and
prune treated the blobs of the snapshots on later pages as unreferenced.
Both now fetch each page through nextListPage, which returns
ErrNoContinuationToken for such a page; prune, snapshot list and remote
nuke already handle a listing error.

The test serves a fixed reply from a stub server, since gofakes3 always
sends the token.

Model: opus-5-5
clawbot force-pushed issue-297-truncated-listing-error from 86ec0df2e7 to 43483e21d4 2026-10-13 07:09:05 +02:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-10-13 07:09:08 +02:00
Some checks are pending
check / check (push) Waiting to run
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-297-truncated-listing-error:issue-297-truncated-listing-error
git checkout issue-297-truncated-listing-error
Sign in to join this conversation.