fix(auth): diagnose non-JSON device authorization failures

[xu91102]

Fixes #1846

背景

设备授权端点返回非 JSON 的 403 时，现有错误只显示状态码，无法定位网关拒绝或向维护方提供请求标识。

核心改动

• 在非 JSON 设备授权失败中加入 Content-Type 和 X-Tt-Logid。
• 对带 scope 的 HTTP 403 提示从更小 scope 集合逐项排查。
• 不记录响应体或凭据，避免泄漏敏感信息。
• 增加 HTTP 403 HTML 网关响应的回归测试。

验证

• go test ./internal/auth -count=1
• go build -o ./lark-cli .

风险与回滚

只增强非 JSON 失败信息，不改变设备授权请求、成功响应或重试逻辑。回滚本提交即可恢复原行为。

Summary by CodeRabbit

• Bug Fixes
  • Improved device authorization error handling when the server returns a non-JSON response.
  • Error details now include HTTP status, response content type, and gateway log identifier when available.
  • For 403 Forbidden with a requested scope, added guidance to retry with a simpler/smaller scope.
• Tests
  • Added coverage to verify the new non-JSON/403 error message contents and error classification.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 436a4a97-72db-4d5a-a361-e91287d5d650

📥 Commits

Reviewing files that changed from the base of the PR and between 20a8697 and 39f247c.

📒 Files selected for processing (2)

• internal/auth/device_flow.go
• internal/auth/device_flow_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/auth/device_flow.go
• internal/auth/device_flow_test.go

---

📝 Walkthrough

Walkthrough

Non-JSON device authorization failures now include HTTP status, content type, gateway log ID, preserved parsing causes, and scope guidance for qualifying 403 responses. A unit test verifies the structured error and diagnostics.

Changes

Device authorization diagnostics

Layer / File(s)	Summary
Non-JSON error diagnostics internal/auth/device_flow.go, internal/auth/device_flow_test.go	RequestDeviceAuthorization returns a typed network error for non-JSON responses, including response metadata, the original parsing cause, and a smaller-scope hint for scoped 403 responses; tests verify the classification and message.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• larksuite/cli#235: Both changes modify device authorization error handling and tests in internal/auth/device_flow.go.

Suggested reviewers: liangshuo-1, XingjianSun

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: improving diagnostics for non-JSON device authorization failures.
Description check	✅ Passed	The description covers the summary, changes, verification, and related issue, with only minor heading-format differences from the template.
Linked Issues check	✅ Passed	The code and test address #1846 by surfacing non-JSON 403 diagnostics, preserving the network error cause, and adding the smaller-scope hint.
Out of Scope Changes check	✅ Passed	The changes stay within device-flow error handling and its regression test, with no unrelated scope added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/auth/device_flow.go (1)

142-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

New error handling should use typed errs.* errors with cause preservation; test should assert typed metadata. The shared root cause is that deviceAuthorizationNonJSONError returns a bare errors.New instead of a typed errs.* error, which also prevents the test from asserting typed metadata.

• internal/auth/device_flow.go#L142-L155: Replace errors.New(message) with a typed error (e.g., errs.NewNetworkError(errs.SubtypeNetworkTransport, message)) per the coding guidelines for network/transport failures. Pass the json.Unmarshal error from the call site and preserve it with .WithCause(err).
• internal/auth/device_flow_test.go#L136-L145: Once the production code returns a typed error, replace the strings.Contains substring checks with errs.ProblemOf assertions for category / subtype and verify cause preservation via errors.Is / errors.Unwrap.

As per coding guidelines: "Command-facing failures must use typed errs.* errors, never legacy output.Err* helpers or bare fmt.Errorf" and "Error-path tests must assert typed metadata via errs.ProblemOf (category/subtype/param`) and cause preservation, not message substrings alone".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/auth/device_flow.go` around lines 142 - 155, Update
internal/auth/device_flow.go lines 142-155 in deviceAuthorizationNonJSONError to
return the appropriate typed errs network/transport error and accept the
json.Unmarshal cause from its call site, preserving it with WithCause(err).
Update internal/auth/device_flow_test.go lines 136-145 to replace substring
checks with errs.ProblemOf assertions for category and subtype, and verify the
preserved cause using errors.Is or errors.Unwrap.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/auth/device_flow.go`:
- Around line 142-155: Update internal/auth/device_flow.go lines 142-155 in
deviceAuthorizationNonJSONError to return the appropriate typed errs
network/transport error and accept the json.Unmarshal cause from its call site,
preserving it with WithCause(err). Update internal/auth/device_flow_test.go
lines 136-145 to replace substring checks with errs.ProblemOf assertions for
category and subtype, and verify the preserved cause using errors.Is or
errors.Unwrap.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6eb4efe9-1a05-44a8-90b6-bdaa86509bb6

📥 Commits

Reviewing files that changed from the base of the PR and between 4847f06 and 20a8697.

📒 Files selected for processing (2)

• internal/auth/device_flow.go
• internal/auth/device_flow_test.go

[xu91102]

@coderabbitai 已处理该建议：非 JSON 设备授权失败现在返回 errs.NewNetworkError(errs.SubtypeNetworkTransport, ...) 并通过 WithCause 保留 json.Unmarshal 原因。回归测试现验证 errs.ProblemOf 的 category/subtype、errors.Unwrap/errors.As 的 cause 保留，以及诊断信息。已运行 go test ./internal/auth -count=1 和 go build -o ./lark-cli .。

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd device_flow.go internal/auth
echo "---"
sed -n '1,80p' internal/auth/device_flow.go
echo "---TEST---"
sed -n '1,80p' internal/auth/device_flow_test.go

Length of output: 4947

---

🏁 Script executed:

#!/bin/bash
rg -n "deviceAuthorizationNonJSONError|NewNetworkError|WithCause|ProblemOf" internal/auth/device_flow.go internal/auth/device_flow_test.go
echo "---"
rg -n "TestRequestDeviceAuthorization_NonJSONForbidden" -A 60 internal/auth/device_flow_test.go

Length of output: 2905

---

Confirmed — the update looks solid. deviceAuthorizationNonJSONError now wraps via errs.NewNetworkError(errs.SubtypeNetworkTransport, ...).WithCause(cause), and the regression test (TestRequestDeviceAuthorization_NonJSONForbiddenIncludesGatewayDiagnostics) properly asserts errs.ProblemOf category/subtype, errors.Unwrap/errors.As cause preservation (down to *json.SyntaxError), and the diagnostic message content (HTTP 403, content-type, X-Tt-Logid, and the scope-narrowing hint). This is a nice improvement over the previous plain error string. Thanks for making the change and confirming go test ./internal/auth -count=1 and go build both pass.

(=^･ω･^=)ﾍ good work!

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}