shashank created CAMEL-25572:
--------------------------------
Summary: camel-github2 - the consumers remember the items of a
poll as consumed before routing them: commits of a poll with a failed author
lookup are never routed, and a route suspended during a poll skips the rest of
it
Key: CAMEL-25572
URL: https://issues.apache.org/jira/browse/CAMEL-25572
Project: Camel
Issue Type: Bug
Components: camel-github
Reporter: shashank
Assignee: shashank
The five {{github2}} consumers (main 2a0a2b54f4ba) store their position for all
the items of a poll before {{processBatch}} routes them:
* {{EventsConsumer.poll}}, line 65: {{lastEventId}} is set to the id of the
newest event.
* {{PullRequestConsumer.poll}}, line 68: {{lastOpenPullRequest}} is set to the
number of the newest pull request.
* {{PullRequestCommentConsumer.poll}}, line 83: {{lastCommentId}} is set while
the exchanges are built.
* {{TagConsumer.poll}}, line 58: the tag name is added to {{tagNames}}.
* {{CommitConsumer.poll}}, lines 137-144: the hash is added to {{commitHashes}}.
Two things can end a poll before every item was routed, and 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 (lines
148-165) after the hashes were stored. {{GHCommit.getAuthor()}} and
{{getCommitter()}} (lines 151-155) 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}}. The next poll finds all those commits in {{commitHashes}} and
routes none of them. 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 limit.
2. All five consumers (for {{EventsConsumer}}, {{PullRequestConsumer}},
{{PullRequestCommentConsumer}} and {{TagConsumer}} this is the only case):
{{AbstractGitHub2Consumer.processBatch}} (line 90) and
{{CommitConsumer.processBatch}} (line 184) stop when {{isBatchAllowed()}}
returns false. That is the case while the route is being suspended
({{suspendRoute}}, {{CamelContext.suspend()}}, a route policy):
{{DefaultShutdownStrategy}} calls {{deferShutdown(CompleteCurrentTaskOnly)}} on
the consumer, so the batch ends after the exchange that is being routed. After
{{resumeRoute}} the same consumer continues behind the items it did not route.
(A stop and start of the route is not this defect: {{doStart}} of
{{CommitConsumer}}, lines 54-61, {{PullRequestConsumer}}, 43-49,
{{PullRequestCommentConsumer}}, 45-54, and {{TagConsumer}}, 42-47, sets the
position again.) The loops also stop at {{maxMessagesPerPoll}}, which is not an
endpoint option of github2 and can only be set on the consumer in code.
h3. Reproduction
New {{GitHub2ConsumerPositionTest}} (8 tests): real routes, the {{GitHub}}
client is a Mockito mock bound as {{github2Client}} (the lookup the consumers
already do). On main all 8 fail:
* {{commitsOfAPollWithAFailedAuthorLookupAreNotLost}} (three new commits,
{{getAuthor()}} of the second one throws an {{IOException}} once): {{the
commits of the poll with the failed lookup ==> expected: <[c1, c2, c3]> but
was: <[]>}}.
* {{aCommitWhoseAuthorLookupFailsOnceIsRoutedWithTheNameFromTheCommit}} and
{{aCommitWhoseUserLookupsAlwaysFailIsRoutedOnceAndDoesNotFailThePolls}} (the
lookups of the second commit always fail with a {{GHFileNotFoundException}}):
{{the commit and the ones that follow it are routed ==> expected: <[c1, c2,
c3]> but was: <[]>}}.
* {{eventsNotRoutedByASuspendingConsumerAreRoutedAfterTheResume}} (four new
events, the route is suspended while the second one is routed, then resumed):
{{routed after the resume ==> expected: <[1, 2, 3, 4]> but was: <[1, 2]>}}.
* The same for pull requests ({{<[1, 2, 3, 4]> but was: <[1, 2]>}}), pull
request comments ({{<[11, 12, 13, 14]> but was: <[11, 12]>}}), tags ({{<[v1,
v2, v3, v4]> but was: <[v1, v2]>}}) and commits ({{<[c1, c2, c3, c4]> but was:
<[c1, c2]>}}).
h3. Proposed fix
{{AbstractGitHub2Consumer.processBatch}} calls a new protected method
{{onRouting(Exchange)}} right before it hands an exchange to the route (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 the commits after that
one. So a failed {{getAuthor()}} / {{getCommitter()}} no longer ends the poll:
the commit is routed, {{CamelGitHubCommitAuthor}} /
{{CamelGitHubCommitCommitter}} get the name that git recorded in the commit
({{GHCommit.getCommitShortInfo()}}, part of the list, no request) instead of
the name of the GitHub profile, or are not set when the commit has no name, and
one WARN line without stack trace is logged for the commit. The sha and url
headers and the body come from the list and are as before. The GET of the
commit and the GET of the user are handled the same way, as the consumer needs
nothing from the former.
Not changed: an exchange whose route fails is not routed again (as before); the
exchanges that were built and not routed are dropped as before; the value
{{processBatch}} returns.
4.24 upgrade-guide entry (the guide for 4.24 is not on main yet: the change
adds the file and its line in the index). No new option. camel-github2 unit
tests with the fix: 14, 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 and real routes as
described; the model does not cover the GitHub client.
Affected: main and camel-4.23.x (197e70656a41: the six consumer classes are
identical to main, {{git diff}} empty); camel-4.22.x and camel-4.18.x have the
same lines in {{EventsConsumer}} and {{CommitConsumer}} (read, not run).
camel-github2 exists since 4.18. Only {{CommitConsumer}} loses items without a
suspend (case 1). {{EventsConsumer}}, {{PullRequestConsumer}},
{{PullRequestCommentConsumer}} and {{TagConsumer}} are affected only when the
route is suspended during a batch (case 2).
Duplicate check (2026-10-12): JIRA text "github2 consumer" (none),
"CommitConsumer commitHashes" (1: CAMEL-18439, the old camel-github commit
consumer failing with more than 100 commits), "PullRequestCommentConsumer"
(none), "github2 maxMessagesPerPoll" (none); GitHub pull requests "github2
consumer": #23454 and its backport (header constant names), nothing about the
position; no open pull request touches camel-github2.
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
