[ 
https://issues.apache.org/jira/browse/CAMEL-26652?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Work on CAMEL-26652 started by Federico Mariani.
------------------------------------------------
> camel-git - the commit, tag and branch consumers mark a failed exchange as 
> consumed and never deliver it again
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-26652
>                 URL: https://issues.apache.org/jira/browse/CAMEL-26652
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-git
>    Affects Versions: 4.14.9, 4.18.5, 4.22.2, 4.23.0
>            Reporter: Federico Mariani
>            Assignee: Federico Mariani
>            Priority: Major
>
> h3. Problem
> The three git consumers ({{type=commit}}, {{type=tag}}, {{type=branch}}) 
> remember an item as consumed after routing it, whether the route succeeded or 
> not. A commit, tag or branch whose exchange failed is never offered again: 
> with the default error handler the message is lost, and only the error 
> handler's log line records it.
> * 
> {{components/camel-git/src/main/java/org/apache/camel/component/git/consumer/AbstractGitConsumer.java}}
>  lines 87-89 (main, commit 7978db3d42dd) call {{onPreProcessed}}, then 
> {{getProcessor().process(exchange)}}, then {{onProcessed(exchange, value)}} 
> unconditionally.
> * A route failure does not make {{process()}} throw: the exception stays on 
> the exchange. {{processBatch}} never reads {{exchange.getException()}} and 
> never calls the consumer's {{ExceptionHandler}}.
> * {{onProcessed}} adds the item to the consumed list: 
> {{GitCommitConsumer.java}} line 82, {{GitTagConsumer.java}} line 63, 
> {{GitBranchConsumer.java}} line 64.
> {code:java}
> Object value = onPreProcessed(exchange);
> getProcessor().process(exchange);
> onProcessed(exchange, value);   // also when the exchange failed
> {code}
> This predates the batch consumer. Up to 4.8 the commit consumer did 
> {{getProcessor().process(e); commitsConsumed.add(commit.getId());}}. The same 
> code is in camel-4.14.9, 4.18.5, 4.22.2, 4.23.0 and main (checked in the 
> source).
> h3. How to reproduce
> Measured on main with a JUnit test:
> * A repository created with JGit in a temporary directory, with 3 commits on 
> {{master}}.
> * The route {{from("git:<dir>?type=commit&branchName=master")}}, whose 
> processor counts the attempts per {{CamelGitCommitId}} and then throws 
> {{IllegalStateException}}. The default error handler is used.
> * The test waits until the 3 commits were attempted, then watches for 3 more 
> seconds, about 6 polls.
> * Control: a 4th commit added afterwards is picked up, so the consumer is 
> still polling.
> Outcome:
> * Each of the 4 commits was attempted exactly once.
> * The log shows {{DefaultErrorHandler - Failed delivery for ... Exhausted 
> after delivery attempt: 1}} once per commit.
> * No commit was offered again.
> h3. Proposed change
> * After {{process()}}, read the outcome. If the exchange failed (an exception 
> that is not handled), do not call {{onProcessed}}, and report the failure 
> through {{getExceptionHandler().handleException(...)}}. This is what 
> camel-mongodb (CAMEL-25024) and camel-couchbase (CAMEL-25171) do now.
> * Read the outcome before the exchange is given back. The exchanges are 
> created with {{createExchange(true)}}, so with the pooled exchange factory 
> they can be reset when their unit of work is done. Create them with 
> autoRelease=false and release them after the outcome has been read, as the 
> camel-mongodb fix does.
> * Risk: a commit that always fails is then routed again on every poll. The 
> poll interval cannot be raised today (see CAMEL-26651), so that one should be 
> fixed first or together. A route that wants to drop a failing commit can 
> still do so with a dead letter channel or a handled {{onException}}.
> * Alternative, if a retry on the next poll is not wanted: keep marking the 
> item consumed, but report the failure through the {{ExceptionHandler}}. The 
> failure is then visible, and {{bridgeErrorHandler}} can route it.
> h3. Impact / upgrade
> * This is a behaviour change: a failed commit, tag or branch is delivered 
> again on the next poll instead of being skipped for good. It needs an entry 
> in the 4.24 upgrade guide.
> * Routes with a dead letter channel or a handled {{onException}} are not 
> affected, because their exchanges complete without an exception.
> * All three consumer types go through {{AbstractGitConsumer.processBatch}}.
> h3. Related
> * See also CAMEL-26651: the scheduler and error handler options of the git 
> consumer are ignored.
> * CAMEL-19665 (commits 51f270663f86 and 83ab5f1536c5) turned the git 
> consumers into batch consumers and kept these semantics.
> * CAMEL-25024 (camel-mongodb) and CAMEL-25171 (camel-couchbase) fixed the 
> same kind of bug.
> * CAMEL-26192 changed {{CamelGitCommitId}} to the sha, which is the value 
> {{onPreProcessed}} keeps.
> _Claude Code on behalf of Federico Mariani_



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

Reply via email to