allthingssecurity opened a new pull request, #27697: URL: https://github.com/apache/camel/pull/27697
# Description [CAMEL-25558](https://issues.apache.org/jira/browse/CAMEL-25558) When the consumer lists the bucket, it adds each listed key to the `inProgressRepository` and then gets the object. If that get fails, the poll fails, and `createExchanges` removes the keys of the exchanges it has already created, but not the key of the object that failed, which has no exchange yet. That key stays in the repository (a memory repository that is only cleared when the endpoint stops), so every later poll skips it as "already in progress": - a listed object that was deleted before the get (another consumer on the bucket, a lifecycle rule, or the exchange of an earlier poll that completed after the listing) fails that poll with `NoSuchKeyException`. The other listed objects are consumed by a following poll, but a new object that is later stored under the same key is never consumed; - an object whose get fails for another reason (access denied, throttling, a network error), or whose download fails with `includeBody=true`, fails that poll in the same way, stays in the bucket, and is never consumed again, until the endpoint is restarted. This is not new and does not come from CAMEL-25521: camel-4.22.x has the same loop and catch block. I found it while comparing this consumer with the other object storage consumers after CAMEL-25521 (#27672). The change, for the listing only: - a failure to get one listed object, or to create its exchange (download of the body, `includeS3Object`), no longer fails the poll. The object is skipped: its stream is closed, its exchange is given back to the exchange factory, and its key is removed from the in-progress repository. The other listed objects are consumed in the same poll, and a later poll tries the skipped object again; - `NoSuchKeyException` is logged at DEBUG, as camel-minio (CAMEL-25512) and camel-google-storage (CAMEL-25519) do for an object that no longer exists. Any other failure is logged at WARN with the key and the exception message, without a stack trace, as camel-ibm-cos does for any failed get (it skips the object and continues). An object that can never be read (archived, access denied) gives one such line at every poll that lists it; - a skipped object is not counted in the polled messages or in `CamelBatchSize`; - the body of `createExchange(ExchangePattern, ResponseInputStream, String)` moved to a private `populateExchange`, so that the exchange is given back when filling it fails (this also covers the `fileName` poll). With the pooled exchange factory it is given back with `done()`: `releaseExchange(exchange, true)` on an exchange that is not done puts it in the pool twice (release, `done()`, the on-done task, release again), and two later objects of the same poll then share one exchange. The pooled test below fails in that way with `releaseExchange`. The same call for exchanges that are never routed exists on main in this consumer (`processBatch` when stopping, and the outer catch) and in other consumers; I did not change those here; - the WARN of the remaining outer catch (now only reached when the in-progress repository itself fails) no longer ends with "This exception is ignored.", as the exception is rethrown. **Decision for the reviewer.** Skip-and-continue for a failed get was chosen to match camel-ibm-cos, and so that one object that cannot be read does not block the bucket: once the key is no longer left in progress, failing the poll would fail every poll for such an object, and the objects listed after it would never be consumed. The cost is that such a failure is no longer a failed poll: it does not reach the exception handler (or the route with `bridgeErrorHandler=true`), does not count for `backoffErrorThreshold`, and when S3 cannot be reached at all each listed object is tried and logged (at most `maxMessagesPerPoll` per poll, 10 by default). If you prefer to keep failing the poll and only remove the key, it is a one-line change (rethrow in the second catch). The listing marker: it is taken from the listing response before the objects are got, as before, and a skipped object does not move it back. When the listing is not truncated, the next poll lists from the start and tries the object again. When it is truncated (more objects than `maxMessagesPerPoll`), the following polls continue with the next pages, and the skipped object is tried again when the listing starts over. `maxMessagesPerPoll` still limits the listing, so a poll with a skipped object routes fewer messages. Not changed, and known limits: - with `fileName`, a failed get still fails the poll (there is nothing else to consume), and its key is removed as before; - an `Error` (for example `OutOfMemoryError` while a large object is read into the body) is not caught, so its key still stays in progress. 4.23 upgrade-guide note under the existing "camel-aws2-s3 - objects in progress in the consumer" entry. Tests, with the in-process `FakeS3Server` of the module (it can now remove an object right after it was listed, refuse the gets of an object with 403 until told otherwise, and cut the body of a get short). The scheduler is not started and the tests call `poll()`; there is no waiting. `AWS2S3ConsumerDeletedObjectTest` (6 tests), and `AWS2S3ConsumerDeletedObjectPooledExchangeTest`, which runs the same 6 with `PooledExchangeFactory` and adds one. All 13 fail without the change in `AWS2S3Consumer`: - `listedObjectThatNoLongerExistsIsSkipped`: `b.txt` is removed after the listing; the poll returns 2 and routes `a.txt` and `c.txt` with `CamelBatchSize` 2. Without the change: `NoSuchKeyException` from `poll()`. - `objectCreatedAgainAfterItWasSkippedIsConsumed` (`includeBody=false`): a new `b.txt` is consumed by the next poll. Without the change: `NoSuchKeyException` from the first poll. - `objectWhoseGetIsRefusedDoesNotStopTheOtherObjects`: every get of `b.txt` is refused (403). The poll returns 2 (`a.txt`, `c.txt`), the next poll consumes a new `d.txt` and tries `b.txt` again, and `b.txt` is consumed once its get is allowed. Without the change: `S3Exception` (Access Denied) from the first `poll()`. - `skippedObjectOfATruncatedListingIsTriedAgainWhenTheListingStartsOver` (`maxMessagesPerPoll=2`, four objects): the first poll returns 1, the second consumes the second page without trying `b.txt`, the third starts over and tries it again. Without the change: `S3Exception` from the first `poll()`. - `objectWhoseDownloadFailsIsSkippedAndNotLeftInProgress`: the content stream fails while it is read into the body; the poll returns 0, the stream is closed and the key removed. Without the change: `RuntimeCamelException` (`IOException: connection reset`) from `poll()`. - `objectStreamIsClosedWhenIncludeS3ObjectFails`: an overridden `includeS3Object` throws; the poll returns 0, the stream is closed and the key removed. Without the change: `IllegalStateException` from `poll()`. - `exchangeOfAnObjectWhoseDownloadFailsIsGivenBackToThePool` (pooled only): the body of `b.txt` is cut short; `a.txt`, `c.txt` and `d.txt` are routed with their own bodies, and the pool statistics show 4 exchanges taken and 4 released, none discarded. Without the change: `RuntimeCamelException` (`ConnectionClosedException: Premature end of Content-Length delimited message body`) from `poll()`. - With the change, the camel-aws2-s3 unit tests pass (98; the 68 integration tests need LocalStack and were skipped). # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [ ] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). (The issue is drafted and not filed yet: `CAMEL-25558` above is a placeholder.) # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested camel-aws2-s3, including the formatter and import-sort plugins; the build did not change any generated file. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
