Skip to content

feat(logs): Limit auto fetch attempts#102286

Merged
Zylphrex merged 11 commits intomasterfrom
txiao/feat/limit-auto-fetch-attempts
Oct 30, 2025
Merged

feat(logs): Limit auto fetch attempts#102286
Zylphrex merged 11 commits intomasterfrom
txiao/feat/limit-auto-fetch-attempts

Conversation

@Zylphrex
Copy link
Copy Markdown
Member

This limits the number of auto fetch attempts made when using the flex time sampling to 10 attempts. After which, the user is prompted to click a button in order to make a few more attempts.

This limits the number of auto fetch attempts made when using the flex time
sampling to 10 attempts. After which, the user is prompted to click a button in
order to make a few more attempts.
@Zylphrex Zylphrex requested a review from a team as a code owner October 29, 2025 02:28
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 29, 2025
cursor[bot]

This comment was marked as outdated.

const limit = autoRefresh ? QUERY_PAGE_LIMIT_WITH_AUTO_REFRESH : QUERY_PAGE_LIMIT;
const shouldAutoFetchNextPage =

const [autoFetchesRemaining, setAutoFetchesRemaining] = useState(maxAutoFetches - 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: autoFetchesRemaining initializes with maxAutoFetches - 1, causing one fewer auto-fetch than intended and a mismatch with the PR description.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The autoFetchesRemaining state is initialized to maxAutoFetches - 1 at static/app/views/explore/logs/useLogsQuery.tsx:675. The shouldAutoFetchNextPage condition checks autoFetchesRemaining > 0, and the counter is decremented before the fetch. This logic results in one fewer auto-fetch than the maxAutoFetches value (e.g., 4 fetches for a maxAutoFetches of 5). This contradicts the PR description, which states "10 attempts", while the default maxAutoFetches is 5, leading to only 4 auto-fetches. Even with resumeAutoFetch, the total is 9 attempts, not 10.

💡 Suggested Fix

Initialize autoFetchesRemaining with maxAutoFetches or change the condition to autoFetchesRemaining >= 0. Alternatively, decrement autoFetchesRemaining after the condition check. Also, align maxAutoFetches default or usage with the stated "10 attempts" from the PR description.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/explore/logs/useLogsQuery.tsx#L675

Potential issue: The `autoFetchesRemaining` state is initialized to `maxAutoFetches - 1`
at `static/app/views/explore/logs/useLogsQuery.tsx:675`. The `shouldAutoFetchNextPage`
condition checks `autoFetchesRemaining > 0`, and the counter is decremented before the
fetch. This logic results in one fewer auto-fetch than the `maxAutoFetches` value (e.g.,
4 fetches for a `maxAutoFetches` of 5). This contradicts the PR description, which
states "10 attempts", while the default `maxAutoFetches` is 5, leading to only 4
auto-fetches. Even with `resumeAutoFetch`, the total is 9 attempts, not 10.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Zylphrex do you want this four or five times?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

a total of 5 queries each batch, but the first batch has this - 1 because the default query isnt counted in the auto fetches

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #102286   +/-   ##
========================================
  Coverage   80.96%    80.96%           
========================================
  Files        8769      8769           
  Lines      389624    389611   -13     
  Branches    24779     24779           
========================================
- Hits       315441    315433    -8     
+ Misses      73805     73800    -5     
  Partials      378       378           

cursor[bot]

This comment was marked as outdated.

Comment thread static/app/views/explore/logs/tables/logsInfiniteTable.tsx Outdated
@Zylphrex Zylphrex merged commit f0f813f into master Oct 30, 2025
47 checks passed
@Zylphrex Zylphrex deleted the txiao/feat/limit-auto-fetch-attempts branch October 30, 2025 14:05
shashjar pushed a commit that referenced this pull request Nov 4, 2025
This limits the number of auto fetch attempts made when using the flex
time sampling to 10 attempts. After which, the user is prompted to click
a button in order to make a few more attempts.

---------

Co-authored-by: Nar Saynorath <nar.saynorath@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
This limits the number of auto fetch attempts made when using the flex
time sampling to 10 attempts. After which, the user is prompted to click
a button in order to make a few more attempts.

---------

Co-authored-by: Nar Saynorath <nar.saynorath@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants