Dhairya-007 opened a new pull request, #29948:
URL: https://github.com/apache/camel/pull/29948

   ## Problem
   
   `Sqs2Consumer.TimeoutExtender` extends the visibility window of in-flight 
messages in batches of ten. `changeMessageVisibilityBatch` reports its failures 
per entry, and the consumer iterates them:
   
   ```java
   if (br.hasFailed()) {
       br.failed().forEach(failedEntry -> {
           if (failedEntry.code().equals(RECEIPT_HANDLE_IS_INVALID)) {
               LOG.debug("Extended visibility window for request entry failed 
with invalid handle.",
                       br.failed());
           } else {
               LOG.warn("Extended visibility window for request entry failed: 
{}", br.failed());
           }
       });
   }
   ```
   
   `failedEntry` is the loop variable and neither branch uses it. Both log 
`br.failed()`, the whole list, so with three failures in a batch the list is 
logged three times over and nothing says which entry each line is about - 
exactly what the log is needed for, since a failed extension means that message 
becomes visible again and will be redelivered.
   
   The `successful()` loop immediately below gets this right and logs 
`successEntry.id()`.
   
   Two smaller things in the same four lines:
   
   - The debug message has no `{}` but is passed `br.failed()`. SLF4J drops a 
trailing argument that has no placeholder unless it is a `Throwable`, and a 
`List` is not, so the invalid-handle case logs the bare sentence and the list 
is thrown away.
   - `failedEntry.code().equals(RECEIPT_HANDLE_IS_INVALID)` dereferences 
`code()`, which is a plain modelled field of `BatchResultErrorEntry` and is not 
required to be set. A reply without it makes the extender thread throw out of 
`forEach`, past the `catch (SdkException e)` which does not cover 
`NullPointerException`, so the remaining batches in that run are skipped.
   
   ## Fix
   
   Log `failedEntry` - its generated `toString` carries `id`, `code`, `message` 
and `senderFault`, so the line identifies the entry and says why it failed - 
give the debug message a placeholder for the entry id, and compare the constant 
with `code()` rather than the other way round.
   
   No behaviour outside logging changes.


-- 
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]

Reply via email to