dengliming commented on code in PR #661:
URL: https://github.com/apache/shenyu-dashboard/pull/661#discussion_r4101067389
##########
src/models/common.js:
##########
@@ -258,7 +258,11 @@ export default {
},
*exportByNamespace(params, { call }) {
+ const { callback } = params;
yield call(asyncConfigExportByNamespace, params);
+ if (callback) {
+ callback();
Review Comment:
[P2] Do not invoke the success callback for an Admin error response
The actual Admin protocol has two response shapes here.
[ConfigsExportImportController.exportConfigsByNamespace](https://github.com/apache/shenyu/blob/master/shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/ConfigsExportImportController.java#L94-L106)
returns ZIP bytes with `Content-Disposition` on success. However,
[ExceptionHandlers](https://github.com/apache/shenyu/blob/master/shenyu-admin/src/main/java/org/apache/shenyu/admin/exception/ExceptionHandlers.java)
returns a JSON `ShenyuAdminResult` for missing permissions (`code: 601`) or
service exceptions (`code: 500`) without setting a non-200 HTTP status.
Authentication failures can also return HTTP 401 through `StatelessAuthFilter`.
`asyncConfigExportByNamespace` currently delegates to `download()`, which
saves any response as a blob and resolves without checking either HTTP status
or the Admin error envelope. This newly added callback therefore closes the
modal and loses the selected namespace even when export failed. The helper
validation gap predates this PR, but closing the modal on that failure is new.
I reproduced the callback firing with the real service/download chain and
mocked HTTP 200 JSON responses `{code: 601, message: "Export failed", data:
null}` and `{code: 500, message: "Export failed", data: null}`.
Please distinguish a successful attachment response from a JSON error,
surface the error and reject/return failure before this callback. Checking
`response.ok` alone is insufficient for the Admin protocol. Add tests that both
HTTP errors and HTTP 200 business-error responses leave the success callback
uncalled.
--
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]
