fix(rocketmq): log warning when FLAG or DELAY header is not numeric

[daguimu]

Describe what this PR does / why we need it

RocketMQMessageConverterSupport#getAndWrapMessage parses the MQ_FLAG and DELAY_TIME_LEVEL Spring Messaging headers into int via Integer.parseInt. The previous implementation wrapped both parses in a single try { ... } catch (Exception ignored) {}, so any malformed header (e.g. a String value injected via a SpEL expression, a typo, or a non-numeric class) was silently absorbed and the producer fell back to flag=0 and delayLevel=0 with no exception, no log line, and no signal to the caller.

The most user-visible consequence is on the delayed-delivery path: a misconfigured DELAY_TIME_LEVEL (e.g. "abc" from a SpEL that returned a String instead of an int) turns a delayed message into an immediate send and the application has no way of knowing.

Does this pull request fix one issue?

NONE

Describe how you did it

• Added a class-level SLF4J logger to RocketMQMessageConverterSupport.
• Extracted the parsing into a parseHeaderAsIntOrDefault(String headerName, @Nullable Object rawValue) helper and parse the two headers independently (flag first, then delayLevel). A malformed value in one header no longer drags the other, well-formed header down to the fallback, and the warning names only the header that actually failed.
• The helper catches NumberFormatException, logs a warn with the header name, the raw value, and the exception message only (no stack trace, to avoid flooding logs on the per-message producer path), and falls back to 0.
• The rawValue parameter is annotated @org.jspecify.annotations.Nullable (the support package is @NullMarked; header lookups can return null).

Describe how to verify it

mvn -pl spring-cloud-alibaba-starters/spring-cloud-starter-stream-rocketmq test — all 16 module tests pass locally, including 6 new ones in RocketMQMessageConverterSupportTest:

• nonNumericDelayTimeLevelHeaderFallsBackToZero — DELAY = "not-a-number" → getDelayTimeLevel() == 0.
• nonNumericFlagHeaderFallsBackToZero — MQ_FLAG = "not-a-number" → getFlag() == 0.
• invalidNumericHeaderDoesNotPropagateException — both headers non-numeric → no exception escapes convertMessage2MQ.
• nonNumericFlagHeaderLogsWarning — uses OutputCaptureExtension to assert the warning contains the header name and the raw value.
• validDelayLevelWithNonNumericFlagIsPreservedAndWarnsOnlyFlag — mixed validity: valid DELAY is preserved, warning names only MQ_FLAG.
• validFlagWithNonNumericDelayLevelIsPreservedAndWarnsOnlyDelayLevel — the symmetric mixed-validity case.

Special notes for reviews

Two intentional behaviour changes relative to the old silent-fallback code, both discussed in review:

1. Mixed-validity fallback changed (improvement). Previously, one malformed header could force both values to 0 (the parses shared one try block). Now each header is parsed independently, so a well-formed header keeps its value even when the other one is malformed. The mixed-validity tests lock this in.
2. Catch narrowed from Exception to NumberFormatException (intentional). String.valueOf(rawValue) can in principle throw if a custom header value's toString() throws; such an exception now propagates instead of being silently converted into flag=0/delayLevel=0. This is deliberate: a throwing toString() is a programming error in the producer application, and silently mapping it to "send immediately with flag 0" is exactly the failure mode this PR removes. NumberFormatException (the only expected failure: non-numeric text) is still handled with warn-and-fallback.

For every previously valid header value the output is unchanged; for a message where all malformed headers are non-numeric text (the common case), the fallback output is also unchanged — the log.warn is the only new externally visible behaviour.

[uuuyuqi]

Thanks for the fix. The direction looks good to me, but I think there are a few details worth adjusting before merging:

1. The warning message may be inaccurate in one mixed-validity case.

The code still parses DELAY_TIME_LEVEL before MQ_FLAG. If DELAY_TIME_LEVEL is valid, for example 2, but MQ_FLAG is non-numeric, delayLevel has already been parsed as 2 before the exception is thrown. After the catch block, the message can still keep delayLevel=2, so the log text falling back to flag=0 and delayLevel=0 is not always true and may mislead troubleshooting. Could we either make the message more generic, or parse/log the two headers independently?

2. The new tests do not verify the main behavior introduced by this PR.

The added tests lock in the existing fallback/no-exception behavior, which is useful, but they would also pass before this change. Since the only new externally visible behavior is the warn log, it would be better to add a log-capture assertion, for example with Spring Boot OutputCaptureExtension, to verify that a malformed header actually emits a warning containing the header name and raw value.

3. Logging the full exception stack trace on every malformed message may be noisy.

This path can run per produced message. If a producer is misconfigured, log.warn(..., e) may emit a full stack trace repeatedly and flood logs. Since the header names and raw values are already included, perhaps logging the exception message/type without the full stack trace would be enough for diagnosis.

[uuuyuqi]

see #4330 (comment)

[uuuyuqi]

Thanks for the update. The latest revision addresses the three points from my previous review:

• parsing the two headers independently makes the warning accurate and preserves the well-formed header when the other one is malformed;
• the new OutputCaptureExtension tests verify the warning behavior and the mixed-validity cases;
• logging only the exception message avoids repeated full stack traces in the producer path.

I found one remaining blocker in the current CI. Integration Testing fails during compilation with:

[NullAway] passing @Nullable parameter `flagObj` where @NonNull is required
[NullAway] passing @Nullable parameter `delayLevelObj` where @NonNull is required

The support package is @NullMarked, while parseHeaderAsIntOrDefault currently declares Object rawValue, even though its Javadoc and null guard say the value may be null. Could you mark it as @Nullable Object rawValue (using org.jspecify.annotations.Nullable) or normalize the values before calling the helper? The test step did not run because compilation failed.

Please also update the PR description, which still describes the previous implementation. In particular, it says the code keeps a shared broad catch (Exception), preserves delay-first parse order, changes only observability, adds three tests, and verifies the warning manually. The current patch parses the headers independently (flag first), catches NumberFormatException, changes the mixed-validity fallback behavior, adds six tests, and verifies logging with OutputCaptureExtension.

One small compatibility note: narrowing the catch from Exception to NumberFormatException means an exception thrown by a custom header value whose toString() fails would now propagate instead of falling back to zero. Please either preserve the previous broad fallback semantics or document that this behavior change is intentional.

Once CI is green and the description is updated, my previous review concerns look resolved.

[uuuyuqi]

LGTM

[uuuyuqi]

Thanks for the update and fix. The same silent catch (Exception ignored) {} still exists on the 2025.0.x branch, so I think this change should be backported there as well.

Could you please open a follow-up PR targeting 2025.0.x and reference this PR as the source? That branch does not currently use @NullMarked, NullAway, or JSpecify, so the backport should keep the independent header parsing, warning behavior, and regression tests, but omit the org.jspecify.annotations.Nullable import and annotation.