Recover LINE auth errors at runtime#188
Conversation
|
Warning Review limit reached
More reviews will be available in 17 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds a generic ChangesLINE Auth Token Recovery
Sequence Diagram(s)sequenceDiagram
participant MatrixEvent as Matrix Event Handler
participant callLineWithRecovery
participant lineClient as line.Client
participant runTokenRecovery
participant LineAPI as LINE API
MatrixEvent->>callLineWithRecovery: callLine(ctx, fn)
callLineWithRecovery->>lineClient: NewClient(accessToken)
callLineWithRecovery->>LineAPI: fn(client) — e.g. SendMessage / React / UploadOBS
LineAPI-->>callLineWithRecovery: auth/token error (code=119 or 401)
callLineWithRecovery->>runTokenRecovery: recoverToken()
runTokenRecovery->>runTokenRecovery: lock recoverMu
runTokenRecovery->>runTokenRecovery: check recoverTime < recentTokenRecoveryWindow
runTokenRecovery->>LineAPI: refreshAndSave() or tryLogin()
LineAPI-->>runTokenRecovery: new access token
runTokenRecovery->>runTokenRecovery: update recoverTime, unlock
runTokenRecovery-->>callLineWithRecovery: recovery ok
callLineWithRecovery->>lineClient: NewClient(newAccessToken)
callLineWithRecovery->>LineAPI: fn(client) retry
LineAPI-->>MatrixEvent: result or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces centralized token/auth recovery for LINE API calls and adds structured helpers to classify auth-related errors, then refactors connector operations to use the new recovery path.
Changes:
- Added
line.IsRefreshRequired,line.IsLoggedOut,line.IsUnauthorizedStatus, andline.IsAuthErrorplus unit tests. - Implemented generic
callLineWithRecovery+LineClient.callLinewrappers and refactored multiple connector handlers to use them. - Added concurrency controls and a short “recent recovery” window to avoid repeated token recovery attempts.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/line/errors.go | Adds helper predicates to detect auth/unauthorized/refresh-required errors. |
| pkg/line/errors_test.go | Adds unit tests for new error classification helpers. |
| pkg/connector/auth_recovery.go | Introduces generic LINE call wrapper with token recovery + retry. |
| pkg/connector/auth_recovery_test.go | Adds tests for retry/recovery behavior and concurrency serialization. |
| pkg/connector/client.go | Routes refresh detection through line helpers and serializes recovery with mutex/window. |
| pkg/connector/send_message.go | Refactors message sending/uploads to use recovery wrapper instead of direct client calls. |
| pkg/connector/userinfo.go | Uses lc.callLine for read receipts. |
| pkg/connector/reaction.go | Uses lc.callLine for reactions and cancellations. |
| pkg/connector/handlers/handler.go | Switches recovery trigger from substring "401" to line.IsUnauthorizedStatus. |
| pkg/connector/handlers/file.go | Retries OBS download after token recovery. |
| .gitignore | Stops ignoring *_test.go so tests can be committed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/line/errors.go (1)
22-24: ⚡ Quick winHarden auth classifiers against case-variant error strings.
These predicates drive token-recovery retry; case-sensitive matching can miss recoverable auth errors if upstream text casing drifts.
Suggested patch
func IsRefreshRequired(err error) bool { if err == nil { return false } - msg := err.Error() + msg := strings.ToLower(err.Error()) return strings.Contains(msg, "\"code\":119") || - strings.Contains(msg, "Access token refresh required") + strings.Contains(msg, "access token refresh required") } func IsLoggedOut(err error) bool { if err == nil { return false } - return strings.Contains(err.Error(), "V3_TOKEN_CLIENT_LOGGED_OUT") + return strings.Contains(strings.ToLower(err.Error()), "v3_token_client_logged_out") } func IsUnauthorizedStatus(err error) bool { if err == nil { return false } - msg := err.Error() - return strings.Contains(msg, "API error 401") || - strings.Contains(msg, "API error 403") || - strings.Contains(msg, "HTTP 401") || - strings.Contains(msg, "HTTP 403") || - strings.Contains(msg, "SSE error: 401") || - strings.Contains(msg, "SSE error: 403") || - strings.Contains(msg, "OBS upload failed (401)") || - strings.Contains(msg, "OBS upload failed (403)") || - strings.Contains(msg, "OBS download failed (401)") || - strings.Contains(msg, "OBS download failed (403)") + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "api error 401") || + strings.Contains(msg, "api error 403") || + strings.Contains(msg, "http 401") || + strings.Contains(msg, "http 403") || + strings.Contains(msg, "sse error: 401") || + strings.Contains(msg, "sse error: 403") || + strings.Contains(msg, "obs upload failed (401)") || + strings.Contains(msg, "obs upload failed (403)") || + strings.Contains(msg, "obs download failed (401)") || + strings.Contains(msg, "obs download failed (403)") }Also applies to: 31-31, 38-48
🤖 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 `@pkg/line/errors.go` around lines 22 - 24, The error string matching in this function is case-sensitive, which can cause recoverable auth errors to be missed if the upstream error messages have different casing. Convert the error message to lowercase using strings.ToLower() before performing the strings.Contains() checks against both the "code":119 pattern and the "Access token refresh required" string. Apply the same case-insensitive matching fix to all other similar error classification predicates mentioned in the file at the referenced line ranges.pkg/connector/auth_recovery.go (1)
24-27: ⚡ Quick winPreserve the triggering auth error when recovery fails.
The returned error currently drops the original LINE call failure context, which makes incident triage harder.
Suggested patch
- return client, zero, fmt.Errorf("failed to recover token after LINE auth error: %w", errRecover) + return client, zero, fmt.Errorf("LINE auth call failed (%v); token recovery failed: %w", err, errRecover)🤖 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 `@pkg/connector/auth_recovery.go` around lines 24 - 27, The error returned when token recovery fails only preserves the recovery error from errRecover but loses the original LINE authentication error that triggered the recovery attempt. Modify the recovery error handling block to capture the original authentication error before calling deps.recover(ctx), and then when errRecover is not nil, wrap both errors together in the returned error using fmt.Errorf or error wrapping to ensure both the original LINE auth error and the recovery error are included in the error chain for better incident triage.
🤖 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.
Inline comments:
In `@pkg/connector/client.go`:
- Around line 33-34: The AccessToken field is read concurrently in
auth_recovery.go (in the newClient closures at Lines 43 and 55) while being
mutated in refresh/login paths, creating a data race condition. Add a new
tokenMu sync.RWMutex field to the LineClient struct to synchronize all token
access. Create a getAccessToken() method that acquires a read lock and returns
the AccessToken value. Update auth_recovery.go to use this getAccessToken()
getter instead of directly accessing AccessToken. Guard all token write
operations in refreshAndSave and tryLogin methods by acquiring a write lock on
tokenMu before modifying AccessToken. This ensures consistent synchronization of
token state across concurrent operations.
---
Nitpick comments:
In `@pkg/connector/auth_recovery.go`:
- Around line 24-27: The error returned when token recovery fails only preserves
the recovery error from errRecover but loses the original LINE authentication
error that triggered the recovery attempt. Modify the recovery error handling
block to capture the original authentication error before calling
deps.recover(ctx), and then when errRecover is not nil, wrap both errors
together in the returned error using fmt.Errorf or error wrapping to ensure both
the original LINE auth error and the recovery error are included in the error
chain for better incident triage.
In `@pkg/line/errors.go`:
- Around line 22-24: The error string matching in this function is
case-sensitive, which can cause recoverable auth errors to be missed if the
upstream error messages have different casing. Convert the error message to
lowercase using strings.ToLower() before performing the strings.Contains()
checks against both the "code":119 pattern and the "Access token refresh
required" string. Apply the same case-insensitive matching fix to all other
similar error classification predicates mentioned in the file at the referenced
line ranges.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a074534e-d455-4510-acf7-d316680eba28
📒 Files selected for processing (11)
.gitignorepkg/connector/auth_recovery.gopkg/connector/auth_recovery_test.gopkg/connector/client.gopkg/connector/handlers/file.gopkg/connector/handlers/handler.gopkg/connector/reaction.gopkg/connector/send_message.gopkg/connector/userinfo.gopkg/line/errors.gopkg/line/errors_test.go
💤 Files with no reviewable changes (1)
- .gitignore
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/reaction.gopkg/connector/handlers/file.gopkg/connector/userinfo.gopkg/line/errors_test.gopkg/connector/auth_recovery_test.gopkg/connector/auth_recovery.gopkg/connector/handlers/handler.gopkg/line/errors.gopkg/connector/client.gopkg/connector/send_message.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/reaction.gopkg/connector/handlers/file.gopkg/connector/userinfo.gopkg/line/errors_test.gopkg/connector/auth_recovery_test.gopkg/connector/auth_recovery.gopkg/connector/handlers/handler.gopkg/line/errors.gopkg/connector/client.gopkg/connector/send_message.go
🔇 Additional comments (9)
pkg/connector/auth_recovery_test.go (1)
21-120: LGTM!Also applies to: 122-133, 135-149, 151-194
pkg/line/errors_test.go (1)
8-84: LGTM!pkg/connector/handlers/handler.go (1)
35-35: LGTM!pkg/connector/send_message.go (1)
34-52: LGTM!Also applies to: 226-228, 240-242, 328-330, 411-413, 421-423, 449-451, 513-515, 657-657, 682-684, 719-719, 746-746, 770-772, 783-785, 837-839, 858-862, 866-869, 879-882
pkg/connector/userinfo.go (1)
40-43: LGTM!pkg/line/errors.go (1)
1-8: Run the required Go hygiene checks for this PR cohort.Please verify formatting, import grouping, and static analysis compliance across changed Go packages (excluding
pkg/ltsm). Run the following checks:
gofmtfor code formattinggoimports -local "github.com/highesttt/matrix-line-messenger"for import groupingrg -n --type=go '\.Msgf\s*\('to scan for prohibitedMsgfusage in zerologgo vetandstaticcheckon all packages exceptpkg/ltsmAs per coding guidelines, ensure compliance with
go fmt,goimportswith local grouping,Msg(notMsgf) for structured logging, and static analysis for all Go code outside the WASM package.pkg/connector/handlers/file.go (1)
41-44: LGTM!pkg/connector/reaction.go (1)
369-372: LGTM!Also applies to: 397-399
pkg/connector/auth_recovery.go (1)
1-8: Run mandatory Go quality checks locally before merging.Per coding guidelines, the following checks are required for all Go files (excluding
pkg/ltsm):
go fmtfor formattinggoimports -local "github.com/highesttt/matrix-line-messenger"for import groupinggo vetfor static analysisstaticcheckfor lintingThese tools must be executed in your development environment to verify compliance before this change is merged.
Summary
Fixes runtime LINE auth recovery for API paths that can currently return expired-token errors directly while the bridge is already running.
When LINE returns an auth error such as
TalkExceptioncode 119 /Access token refresh required, the bridge now attempts token recovery, recreates the LINE client with the recovered token, and retries the failed call once.Fixes #163.
Changes
*_test.goignore so the new auth recovery regression tests are trackedTesting
GOCACHE=/private/tmp/line-gocache GOWORK=off go test ./pkg/line ./pkg/connector ./pkg/connector/handlersdocker run --rm -v "$PWD":/build -w /build alpine:3.23 sh -lc 'apk add --no-cache go git build-base olm-dev staticcheck && go test ./... && go vet $(go list ./... | grep -v /ltsm) && staticcheck $(go list ./... | grep -v /ltsm)'GOCACHE=/private/tmp/line-gocache GOWORK=off go test -race ./pkg/connectordocker build -t matrix-line-auth-recovery .