unbridled-41 opened a new pull request, #4561:
URL: https://github.com/apache/rocketmq-dashboard/pull/4561
Fixes #4557.
## Problem / Evidence
`NotificationOutboxService.sendTestMessage` (behind `POST
/api/settings/general/test-notification`) threw `IllegalArgumentException` for
an unknown channel and `IllegalStateException` for every delivery failure ("No
configured dingtalk webhook", "SMTP is not configured", "DingTalk rejected
webhook: …", "Unable to sign DingTalk webhook"). `GlobalExceptionHandler` has
no handler for either type, so they fell into the generic `Exception` → HTTP
500 with the hard-coded body `Internal Server Error`. The settings page
displays `response.data.message` (`GeneralSettingsTab.sendTest`), so all three
Test buttons reported the same useless 500 for every failure mode and the
diagnostic reason was discarded after `log.error`.
Red output: `expect BusinessException but was: IllegalStateException: Test
notification failed: No configured dingtalk webhook`.
## Root cause / Fix
Error-type mismatch: the service used JDK exceptions while the handler layer
only translates `BusinessException` into an actionable response. Both throws
now carry the reason as a `BusinessException` — 400 `Unsupported notification
channel: <channel>`, 502 `Test notification failed: <reason>` — which the
existing handler renders verbatim and the frontend already surfaces.
## Priority & scoring
PRIORITY 72 = impact 26 (all test-notification failure modes
indistinguishable; operators cannot self-diagnose) + reach 12
(dingtalk/email/sms test buttons on the settings page, a routine ops action) +
repro 20 (curl with an empty webhook) + maintain 14 (aligns the endpoint with
the repo's established error contract). FIX_CONFIDENCE 95 (pure exception-type
change; handler and frontend paths already exist and are tested).
## Tests
- New
`NotificationOutboxServiceTest.testMessageFailureCarriesTheConfigurationReasonTest`:
- Red (baseline): `Tests run: 1, Failures: 1` — expected
`BusinessException` but was `IllegalStateException: Test notification failed:
No configured dingtalk webhook`.
- Green: asserts 502 + message contains "webhook" for the
unconfigured-dingtalk case, and 400 + message contains "fax" for an unknown
channel.
- Full class: `mvn test -Dtest=NotificationOutboxServiceTest` → **26/26**.
- Full backend suite (`SPRING_PROFILES_ACTIVE=dev mvn clean test`): 2396
tests, 0 failures (baseline 2395 + this 1 new test).
## Risk
Low. Status code for a failed delivery changes from 500 to 502 (still an
error status; the body message changes from "Internal Server Error" to the
actionable reason — that is the point). No caller branches on the old 500.
--
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]
