Skip to content

Conversation

@verylovestars
Copy link
Contributor

@verylovestars verylovestars commented Nov 11, 2025

Hi! I found an interesting bug.
When calling InfinityQuery.start(), the error 'getCurrentThrowableError is undefine' occurred. (InfiniteQuery doesn't implement getCurrentThrowableError.)

Here's my proposed solution.

Summary by CodeRabbit

  • New Features

    • Improved error retrieval for infinite queries to make throwable errors accessible.
  • Bug Fixes

    • Fixed start behavior for infinite queries to resolve a stability issue.
  • Tests

    • Added coverage for infinite-query start scenarios, including normal execution and error handling.
  • Chores

    • Added a changeset recording a patch release.

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

Adds tests for InfiniteQuery.start (success and error cases, with a duplicated test block present) and introduces a protected InfiniteQuery#getCurrentThrowableError method that delegates to Query.prototype.getCurrentThrowableError.

Changes

Cohort / File(s) Summary
InfiniteQuery tests
src/infinite-query.test.ts
New test suite(s) for InfiniteQuery.start covering: successful start (calls queryFn, stops loading, marks data fetched, yields one page) and error handling when throwOnError is true. The suite appears duplicated within the file.
InfiniteQuery implementation
src/inifinite-query.ts
Added protected method getCurrentThrowableError(options?: RefetchOptions) that forwards to Query.prototype.getCurrentThrowableError.call(this, options).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as caller (test or consumer)
  participant IQ as InfiniteQuery
  participant Q as Query.prototype

  rect rgba(135,206,235,0.12)
  C->>IQ: call getCurrentThrowableError(options?)
  IQ-->>Q: Query.prototype.getCurrentThrowableError.call(this, options)
  Q-->>IQ: error | undefined
  IQ-->>C: return error | undefined
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review notes:
    • Inspect duplicated test blocks in src/infinite-query.test.ts and remove unintended duplication.
    • Verify filename src/inifinite-query.ts (spelling) matches project conventions and import paths.
    • Ensure the delegated protected method preserves intended access and behavior in inheritance contexts.

Poem

🐰 Tests hop in pairs, one true, one bold,
A tiny method hands errors to old.
Delegation quick, no fluff to borrow,
Fixes stitched in code for a brighter morrow. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely describes the main change: implementing the getCurrentThrowableError method for the InfiniteQuery class.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f938236 and 886114c.

📒 Files selected for processing (3)
  • .changeset/free-things-dream.md (1 hunks)
  • src/infinite-query.test.ts (1 hunks)
  • src/inifinite-query.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/inifinite-query.ts
  • src/infinite-query.test.ts
🔇 Additional comments (1)
.changeset/free-things-dream.md (1)

1-5: Changeset entry format is correct.

The changeset file follows the pnpm changeset convention with proper YAML frontmatter and description. However, the source code files (src/infinite-query.ts and src/infinite-query.test.ts) are not included in this review, so I cannot validate the actual implementation details.

According to the AI summary, the PR includes test duplication—specifically duplicated start-method test blocks in src/infinite-query.test.ts. Please verify that duplicate test code is intentional or consolidate the test cases to avoid redundancy.

Additionally, ensure the following are addressed before merging:

  1. The new getCurrentThrowableError method correctly delegates to Query.prototype.getCurrentThrowableError
  2. Tests comprehensively cover both success and error cases for InfiniteQuery.start()
  3. No test duplication remains in the test file

Would you be able to share the actual implementation files (src/infinite-query.ts and src/infinite-query.test.ts) for a detailed review of the code changes?


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

@verylovestars verylovestars marked this pull request as ready for review November 12, 2025 07:34
@js2me
Copy link
Owner

js2me commented Nov 12, 2025

Hi @verylovestars ! Thanks for your PR!
Please commit your changes using command pnpm changeset

@verylovestars verylovestars force-pushed the fix-infinite-query-start branch from f938236 to 886114c Compare November 13, 2025 07:44
@js2me
Copy link
Owner

js2me commented Nov 13, 2025

Thank you!

@js2me js2me merged commit 26fe358 into js2me:master Nov 13, 2025
2 checks passed
@github-actions github-actions bot mentioned this pull request Nov 13, 2025
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