Skip to content

fix: disable automatic retries for login user mutation#9576

Merged
Cristhianzl merged 3 commits into
mainfrom
cz/remove-retry-login
Aug 29, 2025
Merged

fix: disable automatic retries for login user mutation#9576
Cristhianzl merged 3 commits into
mainfrom
cz/remove-retry-login

Conversation

@Cristhianzl
Copy link
Copy Markdown
Member

@Cristhianzl Cristhianzl commented Aug 27, 2025

Disable automatic retries for login user mutation to prevent multiple authentication attempts and potential rate limiting issues.

Changed: Set retry: false in use-post-login-user.ts

Summary by CodeRabbit

  • Bug Fixes
    • Login requests no longer auto-retry by default, preventing repeated failed attempts and reducing unnecessary wait times.
    • Users receive immediate feedback on failed logins, enabling quicker manual retry or credential correction.
    • Follow-up data refresh after login remains unchanged, preserving existing behavior once authentication succeeds.

@Cristhianzl Cristhianzl self-assigned this Aug 27, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 27, 2025

Important

Review skipped

Auto incremental 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.

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

Walkthrough

Sets a default retry: false for the login mutation hook, placed before spreading user-provided options; existing onSettled behavior and refetch calls remain as-is. No exported signatures change.

Changes

Cohort / File(s) Summary
Auth login mutation options
src/frontend/src/controllers/API/queries/auth/use-post-login-user.ts
Set default retry: false on mutation; caller-supplied options.retry can override via spread. No changes to hook signature or onSettled/refetch logic.

Sequence Diagram(s)

sequenceDiagram
  participant UI as UI
  participant Hook as useLoginUser
  participant API as Auth API
  participant Cache as Query Cache

  Note over Hook: Mutation configured with retry: false (no auto-retries)

  UI->>Hook: mutate(credentials)
  Hook->>API: POST /login
  alt success
    API-->>Hook: 200 OK (tokens/user)
    Hook->>Cache: onSettled → refetch folders, tags
    Hook-->>UI: settled (success)
  else error
    API-->>Hook: 4xx/5xx
    Note over Hook: No automatic retry
    Hook->>Cache: onSettled (still runs)
    Hook-->>UI: settled (error)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cz/remove-retry-login

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions Bot added the bug Something isn't working label Aug 27, 2025
Copy link
Copy Markdown
Collaborator

@lucaseduoli lucaseduoli left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added lgtm This PR has been approved by a maintainer bug Something isn't working and removed bug Something isn't working labels Aug 27, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/frontend/src/controllers/API/queries/auth/use-post-login-user.ts (2)

34-37: Refetch only on success, not onSettled

Avoid refetching folders/tags after failed login attempts.

-      onSettled: () => {
+      onSuccess: () => {
         queryClient.refetchQueries({ queryKey: ["useGetFolders"] });
         queryClient.refetchQueries({ queryKey: ["useGetTags"] });
-      },
+      },

31-36: Ensure retry is always disabled by reordering options

Currently, retry: false is specified before spreading ...options, which means a caller can re-enable retries by passing retry in their options. To guarantee that retries remain disabled, move retry: false after the spread so it cannot be overridden.

• File: src/frontend/src/controllers/API/queries/auth/use-post-login-user.ts
• Lines: around the mutate call’s options object

Suggested diff:

-    {
-      retry: false,
-      ...options,
-      onSettled: () => {
+    {
+      ...options,
+      retry: false,
+      onSettled: () => {
         queryClient.refetchQueries({ queryKey: ["useGetFolders"] });
         queryClient.refetchQueries({ queryKey: ["useGetTags"] });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 61a323d and 264fb9a.

📒 Files selected for processing (1)
  • src/frontend/src/controllers/API/queries/auth/use-post-login-user.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/frontend/src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)

src/frontend/src/**/*.{ts,tsx,js,jsx}: All frontend TypeScript and JavaScript code should be located under src/frontend/src/ and organized into components, pages, icons, stores, types, utils, hooks, services, and assets directories as per the specified directory layout.
Use React 18 with TypeScript for all UI components in the frontend.
Format all TypeScript and JavaScript code using the make format_frontend command.
Lint all TypeScript and JavaScript code using the make lint command.

Files:

  • src/frontend/src/controllers/API/queries/auth/use-post-login-user.ts
🔇 Additional comments (2)
src/frontend/src/controllers/API/queries/auth/use-post-login-user.ts (2)

31-36: Disabling retries by default is good; placement lets callers opt back in

Adding retry: false here clearly communicates intent and prevents accidental multi-attempts; since ...options follows, callers can still override if needed. Note: mutations don’t retry by default in TanStack Query, but an explicit false is useful if global mutation defaults enable retries. (tanstack.com)


31-36: No React Query persistence is configured

I’ve confirmed that the application’s QueryClient is instantiated without any persistence or offline plugins and that there is no usage of PersistQueryClientProvider, resumePausedMutations, or similar APIs anywhere in the codebase. Therefore, failed login mutations cannot be replayed on reconnect, and setting retry: false is sufficient for our auth flow.

– In src/frontend/src/contexts/index.tsx, new QueryClient() is used directly with QueryClientProvider and no persister
– No references to PersistQueryClientProvider, createSyncStoragePersister, or resumePausedMutations were found
– All mutation defaults remain in-memory only, so offline resumes are not possible

This concern can be safely resolved as there is no persistence layer for React Query in the app.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Aug 28, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 28, 2025

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 6%
6.47% (1680/25935) 3.51% (690/19625) 3.47% (194/5584)

Unit Test Results

Tests Skipped Failures Errors Time
682 0 💤 0 ❌ 0 🔥 12.165s ⏱️

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.69%. Comparing base (2313435) to head (70644d7).
⚠️ Report is 6 commits behind head on main.

❌ Your project status has failed because the head coverage (5.81%) is below the target coverage (10.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9576      +/-   ##
==========================================
- Coverage   34.69%   34.69%   -0.01%     
==========================================
  Files        1209     1209              
  Lines       57115    57110       -5     
  Branches     5419     5419              
==========================================
- Hits        19818    19814       -4     
+ Misses      37153    37152       -1     
  Partials      144      144              
Flag Coverage Δ
frontend 5.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ontrollers/API/queries/auth/use-post-login-user.ts 0.00% <ø> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Cristhianzl Cristhianzl added lgtm This PR has been approved by a maintainer and removed lgtm This PR has been approved by a maintainer labels Aug 28, 2025
@Cristhianzl Cristhianzl enabled auto-merge August 28, 2025 02:31
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Aug 29, 2025
@sonarqubecloud
Copy link
Copy Markdown

@Cristhianzl Cristhianzl added this pull request to the merge queue Aug 29, 2025
Merged via the queue into main with commit 1716976 Aug 29, 2025
60 of 62 checks passed
@Cristhianzl Cristhianzl deleted the cz/remove-retry-login branch August 29, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants