Skip to content

fix: bad_credentials after 150s of inactivity#212

Merged
highesttt merged 2 commits into
mainfrom
highest/receive-logout-support
Jul 9, 2026
Merged

fix: bad_credentials after 150s of inactivity#212
highesttt merged 2 commits into
mainfrom
highest/receive-logout-support

Conversation

@highesttt

Copy link
Copy Markdown
Collaborator

No description provided.

@indent-zero

indent-zero Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Adds a 150s idle-timeout to the LINE /operation/receive SSE stream so silent stalls no longer leave the bridge in a broken state until the next send/read-receipt returns 401. On timeout the poll loop probes Talk auth (getLastOpRevision) and either reconnects, delegates to handleReceiveAuthError, runs token recovery, or marks the session logged out immediately — surfacing forced-logout bad_credentials roughly 150s after inactivity instead of much later.

  • pkg/line/sse.go: Add ErrSSEIdleTimeout, defaultSSEIdleTimeout = 150s, and IsSSEIdleTimeout. ListenSSE now installs a single time.AfterFunc per connection that closes the response body on timeout (reset after each successful line read), plus a context.AfterFunc that closes the body on ctx cancel; read errors are classified into idle-timeout / ctx.Err / raw read error.
  • pkg/connector/sync.go: In pollLoop, handle IsSSEIdleTimeout via new handleReceiveIdleTimeout(ctx), which probes getLastOpRevision and branches to markLoggedOutByOtherClient, handleReceiveAuthError, recoverToken, or plain reconnect. localRev is intentionally preserved across the reconnect so the resumed stream replays operations from the stall window.
  • Tests: TestListenSSEIdleTimeout exercises the SSE timeout on an io.Pipe; two new pollLoop tests cover the "probe reveals logged-out" and "probe succeeds, reconnect" branches, with the reconnect test asserting localRev is not advanced from the probe result.

Issues

All clear! No issues remaining. 🎉

4 issues already resolved
  • handleReceiveIdleTimeout discards the fresh revision returned by getLastOpRevisionWithClient on the happy path; seeding localRev from it would let LINE skip re-replaying the ops accumulated during the idle window. (fixed by commit 010bbe9)
  • handleReceiveIdleTimeout accepts the SSE error as _ error and never logs it — either drop the parameter or include it in the probe/recovery log lines so operators can correlate the idle failure with the probe result. (fixed by commit 010bbe9)
  • readSSELine spawns a goroutine and allocates a time.Timer for every SSE line read; a per-connection read deadline (e.g. via the transport or an idle-tracker updated inside the callback) would avoid the per-line churn without changing correctness. (fixed by commit 010bbe9)
  • Comment says "Chrome advertises a 140s polling timeout" but defaultSSEIdleTimeout is 150s — align the doc-comment (or the constant) so future readers know whether the 10s delta is intentional slack over the server-side close. (fixed by commit 010bbe9)

CI Checks

All CI checks passed on commit 010bbe9.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 01929e0f-d276-4468-9fdf-44e2e4c12977

📥 Commits

Reviewing files that changed from the base of the PR and between c9c2f22 and 010bbe9.

📒 Files selected for processing (3)
  • pkg/connector/sync.go
  • pkg/connector/sync_test.go
  • pkg/line/sse.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/connector/sync_test.go
  • pkg/connector/sync.go
📜 Recent review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: Lint with 1.25
  • GitHub Check: build-docker
  • GitHub Check: build-docker
  • GitHub Check: Lint with 1.25
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use go fmt for code formatting across all Go files
Use goimports with -local "github.com/highesttt/matrix-line-messenger" flag to group project-local imports correctly
Use zerolog for logging throughout the codebase
Do not use Msgf in logging; use Msg with structured fields instead
Use Stringer interface where applicable in Go code

Files:

  • pkg/line/sse.go
**/!(ltsm)/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/!(ltsm)/**/*.go: Run staticcheck on all Go files excluding pkg/ltsm package (transpiled WASM code)
Run go vet on all Go files excluding pkg/ltsm package (transpiled WASM code)

Files:

  • pkg/line/sse.go
🔇 Additional comments (3)
pkg/line/sse.go (3)

13-13: LGTM!

Also applies to: 23-27


74-87: LGTM!


108-121: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved streaming reconnect reliability by detecting when no live data arrives within an idle window and retrying appropriately.
    • Prevented unnecessary logged-out/sign-out flows by probing session health on idle-timeout scenarios.
    • More accurately distinguishes temporary connection idleness from genuine authentication problems to reduce disruptions.
  • Tests
    • Added coverage for idle-timeout reconnect behavior, including cases where session health checks succeed vs. indicate logged-out state.

Walkthrough

Adds SSE idle-timeout detection in pkg/line and wires idle-timeout handling into pkg/connector poll/reconnect flow. A new probe path checks auth health after idle SSE stalls and either stops, reuses existing auth handling, or reconnects. Tests cover the idle timeout and probe outcomes.

Changes

SSE Idle Timeout Handling

Layer / File(s) Summary
SSE idle-timeout error and read handling
pkg/line/sse.go, pkg/line/sse_test.go
Adds ErrSSEIdleTimeout, sseIdleTimeout, IsSSEIdleTimeout, and ListenSSE idle/cancellation handling that closes the response body and returns the appropriate error. A test verifies the idle timeout path.
pollLoop idle-probe handling and recovery
pkg/connector/sync.go, pkg/connector/sync_test.go
pollLoop detects IsSSEIdleTimeout, calls handleReceiveIdleTimeout, probes auth health with getLastOpRevisionWithClient, and routes to logged-out handling, receive-auth handling, or reconnect flow. Tests cover logged-out probe failure and successful reconnect.

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

Sequence Diagram(s)

sequenceDiagram
  participant ListenSSE
  participant pollLoop
  participant LineClient
  participant AuthServer

  ListenSSE-->>pollLoop: ErrSSEIdleTimeout
  pollLoop->>LineClient: handleReceiveIdleTimeout(ctx)
  LineClient->>AuthServer: getLastOpRevisionWithClient probe
  alt probe returns errLoggedOut
    LineClient->>LineClient: invalidate token and mark session invalidated
    LineClient-->>pollLoop: stop=true
  else probe succeeds
    LineClient-->>pollLoop: stop=false
    pollLoop->>ListenSSE: reconnect listenSSEWithClient
  end
Loading

Possibly related PRs

  • beeper/line#206: Updates the same pkg/connector/sync.go poll/reconnect flow and related tests.
  • beeper/line#211: Shares the same poll-loop auth handling path used by the new idle-timeout probe logic.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No meaningful PR description was provided to assess against the code changes. Add a brief description of the SSE idle-timeout and reconnect/auth handling changes.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: handling idle inactivity that can trigger bad_credentials errors.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch highest/receive-logout-support

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


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

Comment thread pkg/line/sse.go Outdated
Comment thread pkg/connector/sync.go Outdated
Comment thread pkg/line/sse.go Outdated
Comment thread pkg/connector/sync.go
@highesttt highesttt merged commit 3ff4e26 into main Jul 9, 2026
10 checks passed
@highesttt highesttt deleted the highest/receive-logout-support branch July 9, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant