Skip to content

fix(dashboard): cancel obsolete API requests at the HTTP transport - #852

Open
dr-hoseyn wants to merge 1 commit into
PasarGuard:devfrom
dr-hoseyn:codex/fix-query-request-cancellation
Open

dr-hoseyn wants to merge 1 commit into
PasarGuard:devfrom
dr-hoseyn:codex/fix-query-request-cancellation

Conversation

@dr-hoseyn

@dr-hoseyn dr-hoseyn commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Pass the generated client's AbortSignal through orvalFetcher to ofetch. When a user changes search/page parameters or leaves a screen, TanStack Query already cancels the obsolete query, but the HTTP adapter previously discarded its signal and let the network request finish.

The latest active request still completes normally. This fixes the shared transport used by generated API queries with a two-line change.

Before / after

Local integration comparison against dev (234ab68c), using the actual generated getGetUsersQueryOptions, TanStack QueryObserver, orvalFetcher, and ofetch with a loopback HTTP server:

Scenario / metric Before After
10 successive search keys: HTTP requests started 10 10
Superseded requests canceled at HTTP transport 0 9
Full responses sent by the server 10 1
Synthetic response body bytes sent 655,750 65,575
Unsubscribe after starting all 10: requests canceled 0 10
Unsubscribe after starting all 10: full responses sent 10 0
Already-aborted signal: requests reaching the server 1 0
Already-aborted signal: returned promise Fulfilled Rejected

For each search key, the harness waited until its request reached the server before switching keys. Responses were held until all 10 requests had started. Each response body was 65,575 bytes of synthetic JSON. Cancellation was measured using premature HTTP response-close events, not just canceled query state. The search scenario sent 90% fewer response body bytes in this controlled test; this is not a production performance estimate.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Refactor / cleanup
  • Documentation
  • Tests / CI

Checklist

  • I tested the change locally or explained why it cannot be tested.
  • I added or updated tests for behavior changes.
  • I updated documentation, translations, or examples if needed. (Not applicable.)
  • I checked database migrations when models or schema changed. (Not applicable.)
  • I did not include secrets, tokens, private keys, or unrelated changes.

Testing

  • Ran a standalone local integration harness before and after the patch with Node 24.15.0 and the installed dashboard dependencies. Auth-token/timezone utilities were stubbed; query cancellation and HTTP transport used the real implementation. The harness is a local experiment, not a committed test suite; no test framework was added.
  • Confirmed the latest search still succeeds, canceled requests are not retried, and pre-aborted requests never reach the server after the change.
  • Regression checks passed for GET parameter normalization (object and sort-array inputs), POST JSON bodies, authentication/timezone headers, and HTTP 401 login redirects. Cancellation did not redirect to login.
  • git diff --check: passed.
  • node node_modules/typescript/bin/tsc -p tsconfig.app.json --noEmit --pretty false (from dashboard): fails on existing project diagnostics. A TypeScript compiler API comparison found the same 28 diagnostics before and after, with none in src/service/http.ts.

Suggested manual browser check (not run): throttle the network in DevTools, change user search/page parameters while a request is pending, and check that obsolete requests show as canceled while the final request succeeds. Navigate away with a request pending and check that it is canceled as well.

Screenshots

Not applicable; no visual UI changes.

Notes for reviewers

This forwards the existing optional signal without changing the generated API file or other request options. It is independent of the user/node usage aggregation PRs. HTTP cancellation does not guarantee that backend or database work which has already started is interrupted.

Summary by CodeRabbit

  • New Features
    • Added support for canceling in-progress network requests, helping prevent unnecessary work when requests are no longer needed.

@coderabbitai

coderabbitai Bot commented Sep 3, 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: Team

Run ID: 0478c040-4728-49e6-9873-4b3f96865b5a

📥 Commits

Reviewing files that changed from the base of the PR and between 234ab68 and 23b687d.

📒 Files selected for processing (1)
  • dashboard/src/service/http.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

orvalFetcher now accepts an optional abort signal and forwards it to fetcher, enabling request cancellation.

Changes

Request cancellation

Layer / File(s) Summary
Forward the abort signal
dashboard/src/service/http.ts
orvalFetcher destructures signal and passes it to fetcher in the request options.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 23b68

Requests can now be canceled when they become obsolete, while active requests continue normally. No current merge-blocking risk remains.

Suggested reviewers: normal-dv, x0sina

Poem

A rabbit sends a signal bright
To stop a fetch at just the right time
The request pauses, neat and clear
No stale journey lingers near
Hops complete through code tonight

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: forwarding request cancellation to the HTTP transport to cancel obsolete API requests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@dr-hoseyn

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

1 participant