Skip to content

fix(auth): fail-fast Better Auth session Redis reads - #692

Merged
izadoesdev merged 2 commits into
databuddy-analytics:stagingfrom
Fraol-D:issue/redis-auth-fail-fast
Oct 17, 2026
Merged

izadoesdev merged 2 commits into
databuddy-analytics:stagingfrom
Fraol-D:issue/redis-auth-fail-fast

Conversation

@Fraol-D

@Fraol-D Fraol-D commented Aug 31, 2026 •

Copy link
Copy Markdown

Description

This PR fixes Better Auth session Redis reads so they fail fast when Redis is unavailable, preventing repeated session lookups from waiting for the full Redis timeout.

The change adds a dedicated auth-cache fail-fast path with a 5-second failure window and a 1500ms operation deadline. The auth failure state is kept independent from the existing link-cache failure state, so a failure in one does not affect the other.

Tests were added to verify the fail-fast behavior, independent failure windows, and recovery/reset behavior.

Slice

Checklist
  • This branch started from current staging and does not include another unmerged PR unless it is named above.
  • This is one independently reviewable slice; unrelated cleanup or refactors are in separate PRs.
  • I checked open PRs for overlapping files, contracts, schemas, or deployment configuration and made any dependency explicit above.
  • This PR targets staging; after it closes, this branch will not be reused for another change.
  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by cubic

Makes Better Auth session reads fail fast when Redis is down, so requests don't wait out the full timeout on every session lookup.

Bug Fixes

  • Wraps auth secondary storage reads in runAuthStorageCommand, which rejects immediately for 5 seconds after a recent Redis failure and enforces a 1500ms operation deadline.
  • Keeps the auth fail-fast window separate from the link cache window, and does not clear it on late successes that resolve after a failure.
  • Adds tests covering the fail-fast window, the independent failure state, and recovery once the window expires.

Written for commit 67fe156. Summary will update on new commits.

View guided diff

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

@Fraol-D is attempting to deploy a commit to the Databuddy OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: eee37c3f-481b-4607-934a-229d9099956a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@greptile-apps

greptile-apps Bot commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR is not yet safe to merge because overlapping auth-cache operations can erase a newer failure window and defeat the intended fail-fast behavior.

Findings

  1. P1 Concurrent success clears failure ▶

Summary

This PR adds a dedicated fail-fast path for Better Auth Redis secondary-storage operations.

  • Wraps auth-cache reads and writes with a 1500 ms operation deadline.
  • Opens an independent five-second auth-cache failure window after Redis errors.
  • Adds tests for fail-fast behavior and isolation from the link-cache failure state.
  • No code changed since the previous review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Better Auth secondary-storage operation] --> B{Auth failure window active?}
    B -- Yes --> C[Reject immediately]
    B -- No --> D[Run Redis operation with 1500 ms deadline]
    D --> E{Operation result}
    E -- Failure or timeout --> F[Open five-second failure window]
    E -- Success --> G[Clear failure state]
Loading

Reviews (2) · Last reviewed commit: "fix(auth): fail-fast Better Auth session..."

Comment thread packages/redis/redis.ts Outdated
@izadoesdev

Copy link
Copy Markdown
Member

@greptile review

Please review current head 4f38c6f for correctness and security as part of the requested review of all open PRs. Recheck earlier findings against this head and distinguish unresolved issues from already-fixed feedback.

@izadoesdev izadoesdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the fail-fast idea is sound: Better Auth's findSession does not catch secondary-storage errors, so this turns a 5s x 3 retry stall into a 1.5s failure. Two things before it can merge:

  1. Race on the failure window (Greptile's P1, still present at packages/redis/redis.ts:255): a success resets authCacheFailFastUntil = 0 unconditionally, so an older command that succeeds after a newer one failed clears the fresh 5s window. In runAuthCacheRedisCommand, capture const windowAtStart = authCacheFailFastUntil before the call, and on success only reset it if (authCacheFailFastUntil === windowAtStart).
  2. Rebase onto staging: it now conflicts in packages/auth/src/auth.ts (Better Auth 1.7.5 landed and the SSO plugin was removed) and in packages/redis/000-redis.test.ts (formatting only).

Locally at this head the redis tests pass 8/8, the auth test passes and tsc is clean, so after these two changes it should be close.

@izadoesdev
izadoesdev force-pushed the issue/redis-auth-fail-fast branch from 4f38c6f to 67fe156 Compare October 17, 2026 17:52
@izadoesdev
izadoesdev dismissed their stale review October 17, 2026 18:07

Requested change applied in the rework pushed today: the fail-fast window now expires by time only, so a late success cannot clear it.

@izadoesdev
izadoesdev merged commit d6e7bd3 into databuddy-analytics:staging Oct 17, 2026
8 of 11 checks passed
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.

2 participants