Copilot commented on code in PR #1355:
URL: https://github.com/apache/dubbo-admin/pull/1355#discussion_r2513568910
##########
ui-vue3/src/base/http/request.ts:
##########
@@ -63,27 +63,36 @@ const rejectState: { errorHandler: Function | null } = {
response.use(
(response) => {
NProgress.done()
+ // Success case - code is 'Success'
if (
response.status === 200 &&
- (response.data.code === 200 || response.data.status === 'success')
+ (response.data.code === 'Success')
) {
return Promise.resolve(response.data)
}
+ // Handle 401 unauthorized
if (response.status === 401) {
removeAuthState()
}
- console.error(response.data.code + ':' + response.data.msg)
+ // Show error toast message
+ const errorMsg = `${response.data.code}:${response.data.message}`
+ message.error(errorMsg)
+ console.error(errorMsg)
return Promise.reject(response.data)
},
(error) => {
NProgress.done()
- if (error.response.data) {
- console.error(error.response.data.code + ':' + error.response.data.msg)
+ // Handle error response with data
+ if (error.response?.data) {
+ const errorMsg =
`${error.response.data.code}:${error.response.data.message}`
Review Comment:
The error message field has been changed from `msg` to `message`, but there
are still references to `e.msg` in other parts of the codebase (e.g.,
ui-vue3/src/views/traffic/dynamicConfig/tabs/YAMLView.vue:159,
formView.vue:588, addByFormView.vue:453). This inconsistency will cause these
error handlers to display `undefined` instead of the actual error message.
These files should also be updated to use `.message` instead of `.msg`.
##########
ui-vue3/src/base/http/request.ts:
##########
@@ -63,27 +63,36 @@ const rejectState: { errorHandler: Function | null } = {
response.use(
(response) => {
NProgress.done()
+ // Success case - code is 'Success'
if (
response.status === 200 &&
- (response.data.code === 200 || response.data.status === 'success')
+ (response.data.code === 'Success')
Review Comment:
The success detection logic has been changed from checking numeric code
`200` or status `'success'` to only checking for string code `'Success'`. This
is a potentially breaking change that may cause previously successful responses
to be treated as errors if the backend returns `code: 200` or `status:
'success'`. Verify that all API endpoints have been updated to return `code:
'Success'` for successful responses, or consider maintaining backward
compatibility by keeping both conditions.
##########
ui-vue3/src/base/http/request.ts:
##########
@@ -63,27 +63,36 @@ const rejectState: { errorHandler: Function | null } = {
response.use(
(response) => {
NProgress.done()
+ // Success case - code is 'Success'
if (
response.status === 200 &&
- (response.data.code === 200 || response.data.status === 'success')
+ (response.data.code === 'Success')
) {
return Promise.resolve(response.data)
}
+ // Handle 401 unauthorized
if (response.status === 401) {
removeAuthState()
}
- console.error(response.data.code + ':' + response.data.msg)
+ // Show error toast message
+ const errorMsg = `${response.data.code}:${response.data.message}`
+ message.error(errorMsg)
+ console.error(errorMsg)
return Promise.reject(response.data)
},
(error) => {
NProgress.done()
- if (error.response.data) {
- console.error(error.response.data.code + ':' + error.response.data.msg)
+ // Handle error response with data
+ if (error.response?.data) {
+ const errorMsg =
`${error.response.data.code}:${error.response.data.message}`
Review Comment:
The error message construction lacks null safety checks. If
`response.data.code` or `response.data.message` are undefined or null, the
error message will display 'undefined:undefined' or similar. Consider adding
optional chaining: `const errorMsg = `${response.data.code ??
'Unknown'}:${response.data.message ?? 'An error occurred'}`` or handle missing
fields gracefully.
```suggestion
const errorMsg = `${error.response.data.code ??
'Unknown'}:${error.response.data.message ?? 'An error occurred'}`
```
##########
ui-vue3/src/base/http/request.ts:
##########
@@ -63,27 +63,36 @@ const rejectState: { errorHandler: Function | null } = {
response.use(
(response) => {
NProgress.done()
+ // Success case - code is 'Success'
if (
response.status === 200 &&
- (response.data.code === 200 || response.data.status === 'success')
+ (response.data.code === 'Success')
) {
return Promise.resolve(response.data)
}
+ // Handle 401 unauthorized
if (response.status === 401) {
removeAuthState()
}
- console.error(response.data.code + ':' + response.data.msg)
+ // Show error toast message
+ const errorMsg = `${response.data.code}:${response.data.message}`
Review Comment:
The error message construction lacks null safety checks. If
`response.data.code` or `response.data.message` are undefined or null, the
error message will display 'undefined:undefined' or similar. Consider adding
optional chaining: `const errorMsg = `${response.data.code ??
'Unknown'}:${response.data.message ?? 'An error occurred'}`` or handle missing
fields gracefully.
##########
ui-vue3/src/base/http/request.ts:
##########
@@ -63,27 +63,36 @@ const rejectState: { errorHandler: Function | null } = {
response.use(
(response) => {
NProgress.done()
+ // Success case - code is 'Success'
if (
response.status === 200 &&
- (response.data.code === 200 || response.data.status === 'success')
+ (response.data.code === 'Success')
) {
return Promise.resolve(response.data)
}
+ // Handle 401 unauthorized
if (response.status === 401) {
removeAuthState()
}
- console.error(response.data.code + ':' + response.data.msg)
+ // Show error toast message
+ const errorMsg = `${response.data.code}:${response.data.message}`
Review Comment:
The error message field has been changed from `msg` to `message`, but there
are still references to `e.msg` in other parts of the codebase (e.g.,
ui-vue3/src/views/traffic/dynamicConfig/tabs/YAMLView.vue:159,
formView.vue:588, addByFormView.vue:453). This inconsistency will cause these
error handlers to display `undefined` instead of the actual error message.
These files should also be updated to use `.message` instead of `.msg`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
