[
https://issues.apache.org/jira/browse/CAMEL-25572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-25572:
--------------------------------
Fix Version/s: 4.24.0
> 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
> Priority: Major
> Fix For: 4.24.0
>
>
> 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)
