Skip to content

fix(ui): 修复网络错误时响应拦截器二次报错#701

Open
z2z2qp wants to merge 2 commits into
BeiDouMS:masterfrom
z2z2qp:master
Open

fix(ui): 修复网络错误时响应拦截器二次报错#701
z2z2qp wants to merge 2 commits into
BeiDouMS:masterfrom
z2z2qp:master

Conversation

@z2z2qp

@z2z2qp z2z2qp commented Jun 8, 2026

Copy link
Copy Markdown

No description provided.

@z2z2qp

z2z2qp commented Jun 8, 2026

Copy link
Copy Markdown
Author

fix(ui): 修复网络错误时响应拦截器二次报错

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🤖 AI Code Review

The user wants me to review a pull request for a MapleStory v83 game server project. Let me analyze the diff carefully.

The change is in gms-ui/src/api/interceptor.ts - it's a frontend Vue/TypeScript file that handles axios response interceptors.

Let me look at what changed:

  1. Added a new line: const status = error.response?.status; - This extracts the status code from the error response using optional chaining.

  2. Changed if (error.response.status === 401) to if (status === 401) - Using the previously extracted status variable.

The purpose of this change: If error.response is undefined (which can happen with network errors), accessing error.response.status would throw a TypeError. The new code safely handles this case with optional chaining.

Let me think about potential issues:

  1. Behavioral change: When the error is a "Network Error" (error.response is undefined), status will be undefined. Previously, this code would throw an error when trying to access error.response.status. So this is actually a bug fix.

  2. However, there's a potential UX issue: When there's a network error, the user gets a "无法连接到服务器" message, but the status === 401 check is now undefined === 401 which is false, so the logout callback won't be called. This is actually correct behavior - you don't want to log out the user when there's a network error.

  3. Style: The variable name status is a bit generic. It could be statusCode for better clarity.

  4. One concern: The message handling for different error types. If error.response exists but error.response.data or other fields are used elsewhere, they should also be safely accessed. But looking at the diff, it seems like only .status is accessed.

  5. Security: No security issues with this change.

  6. Potential edge case: What if the server returns a 401 but the error message doesn't match any of the existing conditions? The user would get the default "系统异常" message and then be logged out. This seems fine.

Let me structure my review:

📋 PR 概览

修复了 axios 响应拦截器中 error.response 可能为 undefined 时导致的 TypeError 风险

⚠️ 潜在问题

  • 🟢 低危: 变量名 status 较为通用,建议改为 statusCode 更清晰
  • 这个改动本身是好的,但需要确认其他地方是否也有类似的未保护访问

💡 改进建议

  1. status 改为更具描述性的名称如 statusCode
  2. 考虑在拦截器中统一处理错误状态码,比如将401/403/500等分类处理
  3. 可以考虑使用 axios 的 HTTP_STATUS 常量来比较状态码

📊 评分

  • 代码质量: 4/5
  • 安全性: 4/5
  • 可维护性: 4/5

Let me write a clean, professional review.

📋 PR 概览

修复 axios 响应拦截器中 error.responseundefined 时访问 .status 抛出 TypeError 的潜在 bug,改用可选链提取状态码后再进行 401 鉴权失效判断。

⚠️ 潜在问题

  • 🟢 低危:变量命名不够语义化status 较为泛化,在拦截器上下文中(已存在 error)容易产生歧义,建议改为 statusCodehttpStatus
  • 🟢 低危:副作用差异 — 修复前当网络错误时会在 error.response.status 处直接抛错(用户看到两次 toast 或白屏),修复后网络错误会安静地落入 else 分支。这虽然正确,但建议确认产品上是否需要"网络错误也触发登出回调"的逻辑(一般不需要,当前行为合理)。
  • 🟡 中危:错误处理不够完整 — 当前只判断了 Network Error 和默认情况,缺少对 error.response.status 为 4xx/5xx 时的分类提示(如 403 权限不足、500 服务器异常),用户得到的永远是"系统异常",体验不佳。

💡 改进建议

  1. 重命名 + 常量化:将 status 改为 statusCode,并考虑用 const HTTP_UNAUTHORIZED = 401; 替代魔法数字,提升可读性。
  2. 补全 HTTP 状态码分支
    if (status === 401) { /* 登出 */ }
    else if (status === 403) { errorMessage = '没有权限'; }
    else if (status && status >= 500) { errorMessage = '服务器异常,请稍后再试'; }
  3. 统一错误入口:建议在拦截器外层包一层 try/catch,防止后续扩展时再次出现 Cannot read property of undefined 这类低级错误。

📊 评分

维度 评分
代码质量 4 / 5
安全性 4 / 5
可维护性 4 / 5

总评:这是一个小而正确的防御性编程改动,方向完全正确 ✅。代码虽短但避免了一个真实的运行时崩溃,属于"少改一行,多睡一晚"的典型范例。后续可围绕错误分类与统一处理继续完善。

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🤖 AI Code Review

The user wants me to review a PR for a Java game server (MapleStory v83 style), but the diff is actually for a TypeScript/React frontend file (gms-ui/src/api/interceptor.ts). This is the GMS (Game Management System) UI - the admin panel for managing the game server, not the game server itself.

Let me analyze the changes:

The change is in an axios response interceptor. Before:

  • It accessed error.response.status directly in the 401 check, which would throw a TypeError if error.response was undefined (e.g., for the 'Network Error' case that's handled earlier).

After:

  • It extracts status from error.response?.status using optional chaining, defaulting to undefined if response doesn't exist.
  • Then uses this status variable for the 401 check.

Issues to identify:

  1. There's a potential bug: status is declared with const but only initialized with a value. In TypeScript strict mode, const variables must be initialized. If error.response?.status is undefined, then status will be undefined. The let errorMessage; follows the same pattern though.

  2. Actually, looking more carefully - const status = error.response?.status; is fine syntactically. It will be undefined if error.response is undefined.

  3. The logic flow:

    • Network Error case → shows message → falls through to 401 check (but now status is undefined, so 401 check is skipped, good!)
    • Other errors → shows message → checks 401

    Wait, before the fix, when error.message === 'Network Error', accessing error.response.status would throw an error because there's no response for a network error. So this is actually a bug fix!

  4. TypeScript: const with possibly undefined value - might need a type annotation or assertion. But let errorMessage; above also has this issue, so it's consistent with existing code style.

  5. Minor: The variable is hoisted to the top of the error handler but only used in the 401 check. This is fine.

Let me provide a structured review.

📋 PR 概览

修复 axios 响应拦截器中 error.responseundefined 时(如 Network Error 场景)访问 .status 导致的潜在 TypeError。

⚠️ 潜在问题

🟢 低危

  • 变量提升与作用域status 在函数顶部声明,但仅在末尾的 401 分支使用,中间无逻辑关联。建议内联到使用处(error.response?.status === 401),可读性更好。
  • 类型不明确status 推导为 number | undefined,建议显式标注 const status: number | undefined = ...,便于维护时理解此处依赖可选链。

💡 改进建议

  1. 直接使用可选链内联判断:无需中间变量,改写为:

    if (error.response?.status === 401) {

    既消除冗余变量,又保持与下方 error.response?.status 风格一致。

  2. 统一 errorMessage 的处理模式:当前 let errorMessage; 在函数体中先声明再赋值,若后续需扩展错误类型(如 403、500),建议改为 switch (error.response?.status) 或查表方式,便于维护。

  3. Network Error 分支应直接 return:当前网络错误时仍会继续执行到 401 判断逻辑(虽然现在 status 为 undefined 不会误触发,但流程不直观)。建议在网络错误/超时分支后 return Promise.reject(error),避免继续往下走。

📊 评分

维度 评分 说明
代码质量 ⭐⭐⭐⭐ 修复了一个真实存在的潜在 bug,逻辑正确
安全性 ⭐⭐⭐⭐⭐ 无安全风险
可维护性 ⭐⭐⭐ 功能正确但变量使用可优化

总结:这是一个正确的 bug 修复 PR。改动前,当 error.message === 'Network Error' 时(没有 response 对象),执行 error.response.status 会抛出 Cannot read properties of undefined,现在通过可选链安全地规避了这个问题。改动质量良好,建议按上述建议进一步优化。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant