unbridled-41 opened a new pull request, #5938:
URL: https://github.com/apache/rocketmq-dashboard/pull/5938
### Which Issue(s) This PR Fixes
Fixes #5937
### Problem / Evidence
The loop's stated purpose is per-id isolation, but it only catches
`BusinessException`:
```java
// NotificationOutboxService.retryFailedDeliveries (before)
try { retryFailedDelivery(deliveryId); succeeded.add(deliveryId); }
catch (BusinessException error) { failures.put(deliveryId,
error.getMessage()); }
```
Each id is committed by its own `mapper.update` before the next iteration,
so a `RuntimeException` on a later row escapes and 500s the request while
earlier rows are already `PENDING`:
```
NotificationOutboxServiceTest#bulkRetryKeepsTheOutcomeOfEarlierDeliveriesWhenALaterOneFailsUnexpectedlyTest
org.springframework.dao.DataAccessResourceFailureException: db down
(escapes the call)
```
The page shows the bulk-retry error and does not reload, so those rows keep
displaying `FAILED` until the next refresh although the dispatcher will deliver
them.
### Root cause / Fix
Unexpected failures were treated as fatal for the whole request. Catch
`RuntimeException` per item as well, record it beside the business failures
(message, or the exception class when it carries none), and keep the loop's
guarantee - the pattern `InstanceService.deleteInstances` and
`AlertService.bulkDeleteRules` already use.
### Priority and scoring
**PRIORITY 48** — impact 16/40 (a partially applied action reported as a
total failure; the table then contradicts the outbox), blast radius 10/20 (bulk
retry after an infrastructure hiccup), reproducibility 18/20 (deterministic
with one failing id, pinned by the new test), maintenance value 4/20.
**FIX_CONFIDENCE 85** — one catch clause, with two precedents in the same
codebase.
### Tests
`cd server && mvn -o -B -ntp test
-Dtest='org.apache.rocketmq.studio.ops.alert.**'` -> `Tests run: 327, Failures:
0, Errors: 0`.
| Test | Before | After |
|---|---|---|
|
`bulkRetryKeepsTheOutcomeOfEarlierDeliveriesWhenALaterOneFailsUnexpectedlyTest`
| FAIL (the exception escapes the call) | PASS (`succeededIds=[8]`, `failures`
contains 9) |
`mvn -o -B -ntp checkstyle:check` passes.
### Risk
Only the failure bookkeeping changes: a request that used to fail wholesale
now reports which ids succeeded, which is what the response type already models.
--
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]
