yyqdbngt opened a new pull request, #4691:
URL: https://github.com/apache/rocketmq-dashboard/pull/4691
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Brief Description
`ToolExecutionContext.convertInput` handed the caller's arguments straight
to Jackson. Every
numeric tool argument is declared as `type: integer` in the input schemas,
and JSON Schema accepts
*any* integral number there, so an argument the tool cannot represent —
`1e30`,
`9223372036854775808`, or `5000000000` for an `int` component such as
`page.pageSize` — passed
input validation and then failed inside the conversion with a raw
`IllegalArgumentException`:
```
java.lang.IllegalArgumentException: Numeric value (1.0E30) out of range of
long
```
`ToolInvocation#invoke` calls `convertInput`, and
`ToolExecutionService#executeInternal` maps that
`RuntimeException` to `ToolError.UNEXPECTED_EXECUTION_FAILURE`, i.e. HTTP
500 / `INTERNAL_ERROR`
with the hint "Review the failure message; no specific recovery action is
available." — the caller's
own bad argument was reported as a server fault. The same class of defect
was fixed before for the
message-query tool ("bound timestamp arguments in message query tool",
#2311): unrepresentable
numbers must come back as HTTP 400, not as a 500.
`convertInput` now
- rejects a numeric argument that does not fit a signed 64-bit value, naming
the argument
(`Invalid tool parameter: startTime`, `... : page.pageSize`), and
- translates any remaining binding failure of the (already schema-validated)
arguments into
`INVALID_ARGUMENT` instead of letting it escape as an internal error.
Representable arguments are untouched, including an integral float such as
`1.7e12`, which still
binds to `1700000000000`.
### How Did You Test This Change?
New test `ToolExecutionContextTest` drives the real entry point
(`convertInput` with the same
argument types the JSON layer produces: `Double`, `BigInteger`, `Long`) for
a float outside the
`long` range, a `BigInteger` outside it, and a nested `int` overflow, plus a
guard case proving
representable numbers still convert.
Before the fix (red):
```
$ cd server && mvn -B -ntp test -Dtest=ToolExecutionContextTest
[ERROR] Tests run: 4, Failures: 3, Errors: 0, Skipped: 0
[ERROR]
ToolExecutionContextTest.convertInputRejectsAFloatArgumentOutsideTheLongRangeTest:55
Expecting actual throwable to be an instance of:
...ToolExecutionException
java.lang.IllegalArgumentException: Numeric value (1.0E30) out of
range of long (-9223372036854775808 - 9223372036854775807)
[ERROR]
ToolExecutionContextTest.convertInputRejectsAnIntegerArgumentOutsideTheLongRangeTest:71
java.lang.IllegalArgumentException: Numeric value
(9223372036854775808) out of range of long (...)
[ERROR]
ToolExecutionContextTest.convertInputRejectsANestedArgumentOutsideTheIntRangeTest:84
java.lang.IllegalArgumentException: Numeric value (5000000000) out
of range of int (-2147483648 - 2147483647)
```
(the fourth test, `convertInputKeepsRepresentableArgumentsTest`, already
passed before the fix)
After the fix (green) — the whole tool package, so the neighbouring handler
and contract tests are
covered as well:
```
$ cd server && mvn -B -ntp test
-Dtest='org.apache.rocketmq.studio.ops.ai.tool.**.*Test'
[INFO] Tests run: 157, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
`mvn test` runs checkstyle in the `validate` phase: `You have 0 Checkstyle
violations.`
Note on the full suite: on a clean `rocketmq-studio` checkout `mvn -B -ntp
test` already reports
`Tests run: 3051, Failures: 6, Errors: 25, Skipped: 4`. The 11 red classes
are the MySQL 8 backed
Spring integration tests (`AuthServiceBootstrapIntegrationTest`,
`AuthServiceConcurrencyIntegrationTest`,
`AuthServiceSessionOverviewIntegrationTest`,
`HealthProbeIntegrationTest`, `QueryHistoryServiceIntegrationTest`,
`NativeAlertEvaluationTransactionTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`) plus the
external-CLI ones
(`CliAgentProviderTest`, `ClaudeCodeAgentProviderTest`). None of them are
touched by this change.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(no UI text in this change)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header
- [x] Documentation touched where behaviour changed (no user-visible
contract change: the schemas already declared these arguments, the error class
they produce is what changed)
--
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]
