allthingssecurity opened a new pull request, #27756: URL: https://github.com/apache/camel/pull/27756
# Description [CAMEL-25572](https://issues.apache.org/jira/browse/CAMEL-25572) The five `github2` consumers stored their position for all the items of a poll before `processBatch` routed them: `EventsConsumer` set `lastEventId` to the newest event, `PullRequestConsumer` set `lastOpenPullRequest` to the newest pull request, `PullRequestCommentConsumer` set `lastCommentId` while it built the exchanges, `TagConsumer` added the names to `tagNames` and `CommitConsumer` added the hashes to `commitHashes`. Two things can end a poll before every item was routed. In both cases the items that were not routed are behind the position, so the following polls skip them. That part is silent: in case 1 the failed poll itself is logged at WARN by `ScheduledPollConsumer` ("Failed polling endpoint ... Will try again at next poll"), but nothing says that the commits of that poll are never delivered afterwards. 1. `CommitConsumer` only, default options: the exchanges are built after the hashes were stored, and `GHCommit.getAuthor()` / `getCommitter()` do requests of their own. In github-api 1.330 (the version in `parent/pom.xml`) `getAuthor()` calls `populate()`, a GET of `commits/<sha>` once for each listed commit, and then `GitHub.getUser(login)`, a GET of `users/<login>` unless the login is cached. When one of them fails, the poll ends with the `IOException` before `processBatch`, and the next polls find all the new commits of that poll in `commitHashes`. What makes them fail: a 404 for the commit or the user (for example a deleted account), a server error, or a network error after the two connection retries of the client. A rate limit does not, with the client Camel creates: `GitHubClientFactory` uses the `GitHubBuilder` defaults (`RateLimitHandler.WAIT` / `AbuseLimitHandler.WAIT`), which sleep and retry. It does with a client from the registry that is configured to fail on a rate li mit. 2. All five consumers (for `event`, `pullRequest`, `pullRequestComment` and `tag` this is the only case): `processBatch` stops when `isBatchAllowed()` returns false, which is the case while the route is being suspended (`suspendRoute`, `CamelContext.suspend()`, a route policy). After the resume the same consumer continues behind the items it did not route. A stop and start of the route is not changed here: `doStart` of `CommitConsumer` (lines 54-61), `PullRequestConsumer` (43-49), `PullRequestCommentConsumer` (45-54) and `TagConsumer` (42-47) sets the position again. The loop also stops at `maxMessagesPerPoll`, which is not an endpoint option of github2 and can only be set on the consumer in code. This change: `AbstractGitHub2Consumer.processBatch` calls a new protected method `onRouting(Exchange)` right before it hands an exchange to the route (a no-op by default, so a custom subclass still compiles). The five consumers move their position there, one exchange at a time, and no longer in `poll`. `CommitConsumer` already set `lastSha` at that point in its own copy of `processBatch`; the copy is removed, and `lastSha` and `commitHashes` are updated in `onRouting`. With the position moved only at routing, a lookup that always fails (the 404 of a deleted account) would fail every poll and hold back all the commits after that one until it leaves the 100 commits a poll lists. So the second part of this change: a failed `getAuthor()` / `getCommitter()` no longer ends the poll. The commit is routed, and - `CamelGitHubCommitAuthor` / `CamelGitHubCommitCommitter` get the name that git recorded in the commit (`getCommitShortInfo().getAuthor()` / `getCommitter()`, which is part of the list of commits and needs no request) instead of the name of the GitHub profile; the header is not set when the commit has no name either. A commit whose author has no GitHub account has no such header, as before. - `CamelGitHubCommitSha`, `CamelGitHubCommitUrl` and the body (the message, or the `GHCommit`) are as before; they come from the list. - One WARN line without stack trace is logged for the commit. It is not repeated by the following polls, as the commit is routed and then not read again (it can be once more when a suspend ended the batch before the commit was routed). The public API of github-api does not tell whether the GET of the commit or the GET of the user failed, and the consumer needs nothing from the GET of the commit, so both are handled the same way. If a list entry came without its `commit` object and it could not be loaded either (GitHub does not send such entries), the commit is routed with the sha and url headers and a null message body; it is not retried, so there is no loop. On main that case failed the poll. Not changed: an exchange whose route fails is not routed again; the exchanges that were built and not routed are dropped; the value `processBatch` returns. There is an entry in the 4.24 upgrade guide. The guide for 4.24 is not on main yet, so this change adds the file and its line in the index. Tests: - New `GitHub2ConsumerPositionTest` (8 tests) with real routes; the `GitHub` client is a Mockito mock bound as `github2Client`, the registry lookup the consumers already do. Five tests suspend the route from another thread while the second of four new items is routed, wait until `isBatchAllowed()` is false, and resume the route. Three are about the lookup, each with three new commits: `getAuthor()` of the second one fails once (routed with the git author name, the committer from the lookup that worked, the lookup is not repeated); `getAuthor()` and `getCommitter()` always fail with a `GHFileNotFoundException` (routed once, git author name, no committer header, no failed poll counted by a poll strategy, one lookup each over the following polls); and the loss as it was found (`getAuthor()` throws an `IOException` once, all three commits are routed once). - Without the change all 8 fail: `expected: <[c1, c2, c3]> but was: <[]>` for the three lookup tests, and for the suspended routes `expected: <[1, 2, 3, 4]> but was: <[1, 2]>` (events, pull requests), `<[11, 12, 13, 14]> but was: <[11, 12]>` (comments), `<[v1, v2, v3, v4]> but was: <[v1, v2]>` (tags), `<[c1, c2, c3, c4]> but was: <[c1, c2]>` (commits). - With the change, the camel-github2 unit tests pass: 14 tests, 0 failures. The integration tests need a GitHub token and were not run. How it was found: a small TLA+ model of a poll gave a 4-state trace (one new item is read, building the exchange fails) and a 6-state trace (the route is suspended during the batch) in which the consumer is past an item it did not route. Both were then reproduced with the real consumers in the tests above, which are the evidence; the model does not cover the GitHub client. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] 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). # 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 the affected module, including the formatter and import-sort plugins. 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]
