Skip to content

refactor: replace axios with native fetch [P1.04]#120

Closed
cesarnml wants to merge 2 commits into
agents/p1-03-eslint-plugin-svelte3-eslint-plugin-svelte-full-migrationfrom
agents/p1-04-replace-axios-with-native-fetch
Closed

refactor: replace axios with native fetch [P1.04]#120
cesarnml wants to merge 2 commits into
agents/p1-03-eslint-plugin-svelte3-eslint-plugin-svelte-full-migrationfrom
agents/p1-04-replace-axios-with-native-fetch

Conversation

@cesarnml
Copy link
Copy Markdown
Owner

@cesarnml cesarnml commented May 2, 2026

Summary

  • delivery ticket: P1.04 Replace axios with native fetch
  • ticket file: docs/02-delivery/phase-01/ticket-04-axios-to-fetch.md
  • stacked base branch: agents/p1-03-eslint-plugin-svelte3-eslint-plugin-svelte-full-migration
  • self-audit: outcome clean completed at 2026-05-02 08:16 UTC

External AI Review

  • vendor: Qodo · findings: 4 · patched: 2 · deferred: 1 · rejected: 1
  • Patched — WakaTime durations missing response.ok check (src/routes/api/wakatime/current/durations/+server.ts): Non-2xx WakaTime responses were silently treated as success and returned to the cron pipeline. Added if (!response.ok) throw error(...) before .json(). Commit 28c08bf.
  • Patched — loading.off() not called on fetch error (src/routes/+page.svelte): onWakaRange left the loading store stuck on network/parse failure. Wrapped in try/finally. Commit 28c08bf.
  • Deferred — no spellcheck CI: Cross-cutting concern; deferred to a dedicated ticket.
  • Rejected — CLAUDE.md axios reference: The reference in the State management section is addressed in PR docs(phase-01): closeout ticket - mark complete, update CLAUDE.md, roadmap, and plan [P1.06] #122.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
coding-stats Ready Ready Preview, Comment May 2, 2026 10:10am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 2, 2026

Important

Review skipped

Ignore keyword(s) in the title.

⛔ Ignored keywords (9)
  • docs
  • chore
  • style
  • refactor
  • perf
  • test
  • build
  • ci
  • revert

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 YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: fcbb7620-fc4a-4d78-b132-97ec6fe3a961

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

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 and usage tips.

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented May 2, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Durations errors bypass catch🐞 Bug ≡ Correctness
Description
src/routes/api/wakatime/current/durations/+server.ts parses response.json() without checking
response.ok, so non-2xx WakaTime responses won’t throw and the try/catch won’t run. The cron job
then writes durationsResult.data into Supabase, which can be undefined when the upstream payload
isn’t a DurationsResult.
Code

src/routes/api/wakatime/current/durations/+server.ts[R15-18]

+    const durationsResult: DurationsResult = await fetch(
    `${BaseUrl.WakaTime}${RestResource.Durations}?api_key=${WAKA_API_KEY}&date=${date}&slice_by=${slice_by}`,
-    )
+    ).then((response) => response.json())
+
Evidence
The durations route always returns json(durationsResult) from response.json() with no
ok/status validation, while the cron durations pipeline blindly dereferences
durationsResult.data and persists it. This creates a direct path for non-DurationsResult
payloads (e.g., error JSON) to be stored as if they were valid durations data.

src/routes/api/wakatime/current/durations/+server.ts[9-22]
src/routes/api/cron/wakatime/durations/+server.ts[7-30]
src/lib/constants.ts[109-124]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`fetch()` does not reject on HTTP error statuses, so the WakaTime durations route currently treats non-2xx responses as success and returns/parses them as `DurationsResult`. This defeats the route’s `try/catch` intent and allows invalid payloads to propagate into the cron pipeline that persists `durationsResult.data`.
### Issue Context
The cron handler at `src/routes/api/cron/wakatime/durations/+server.ts` calls `ApiEndpoint.Durations` and persists `durationsResult.data` into Supabase without validating shape.
### Fix Focus Areas
- src/routes/api/wakatime/current/durations/+server.ts[9-22]
- src/routes/api/cron/wakatime/durations/+server.ts[7-30]
### Implementation notes
- Switch to `const response = await fetch(...)`.
- If `!response.ok`, throw `error(response.status, ...)` (or throw a normal error) before calling `response.json()` so failures don’t serialize into a “successful” payload.
- Only parse JSON into `DurationsResult` after the status check.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Docs changed without spellcheck 📘 Rule violation ✧ Quality
Description
This PR adds/edits natural-language documentation text, but the repo provides no evidence that an
automated spellcheck was run for these changes (no spellcheck script or CI step). This risks
shipping typos and violates the documented spellcheck requirement for docs/user-facing text updates.
Code

docs/02-delivery/phase-01/ticket-04-axios-to-fetch.md[R48-51]

+Red first: `pnpm check` passed before implementation with the existing warning baseline.
+Why this path: Replaced each axios call with native `fetch(...).then((response) => response.json())`, keeping the same direct JSON assignment shape the ticket called for and removing the axios-only component test mock.
+Alternative considered: A shared fetch helper would reduce repetition, but this ticket intentionally avoids adding new error-handling behavior or broadening the abstraction surface.
+Deferred: HTTP status handling remains unchanged in spirit and is not added here; Codex preflight is disabled per operator instruction because the current environment cannot run the Claude-code-only preflight agent.
Evidence
PR Compliance ID 474682 requires that an automated spellchecker be run for changed docs/user-facing
text. The PR updates the ticket doc’s ## Rationale text, but the CI workflow and npm scripts shown
do not include any spellcheck execution, so there is no verifiable evidence the spellchecker ran for
these changes.

Rule 474682: Run spellcheck on docs and user-facing text changes
docs/02-delivery/phase-01/ticket-04-axios-to-fetch.md[48-51]
.github/workflows/ci.yaml[12-54]
package.json[5-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Docs/user-facing text changed in this PR, but there is no verifiable evidence an automated spellcheck ran (no CI step and no package script).
## Issue Context
A `cspell.json` exists, but the current CI workflow runs `pnpm format` only and `package.json` has no spellcheck script; compliance requires spellcheck coverage for changed docs/text.
## Fix Focus Areas
- .github/workflows/ci.yaml[12-54]
- package.json[5-36]
- docs/02-delivery/phase-01/ticket-04-axios-to-fetch.md[48-51]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Loading not cleared on error🐞 Bug ☼ Reliability
Description
onWakaRange() calls loading.on() and then awaits fetch(...).json() but does not use
try/finally. If the request or JSON parsing throws, loading.off() is skipped and the UI can
remain stuck in a loading state.
Code

src/routes/+page.svelte[R32-38]

 const onWakaRange = async () => {
   loading.on()
-    const { data } = await axios.get(`${ApiEndpoint.SupabaseSummaries}?range=${$selectedRange}`)
-    summaries = data
+    summaries = await fetch(`${ApiEndpoint.SupabaseSummaries}?range=${$selectedRange}`).then(
+      (response) => response.json(),
+    )
   loading.off()
 }
Evidence
loading.off() is only called on the success path; there is no finally/cleanup branch. The loading
store exposes explicit on/off methods, so skipping off() leaves the boolean true.

src/routes/+page.svelte[32-38]
src/lib/stores/loading.ts[3-11]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`onWakaRange()` does not guarantee `loading.off()` runs when fetch/json fails.
### Issue Context
Any thrown error before `loading.off()` (network error, JSON parse error, etc.) leaves the store stuck in the loading state.
### Fix Focus Areas
- src/routes/+page.svelte[32-38]
### Suggested change
Wrap the fetch/json assignment in `try { ... } finally { loading.off() }`, and optionally handle/display the error.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Docs still mention axios 🐞 Bug ⚙ Maintainability
Description
CLAUDE.md still claims selectedRange changes trigger an axios.get, which is now incorrect
after switching the page to use fetch. This can mislead future debugging and onboarding.
Code

src/routes/+page.svelte[R34-36]

+    summaries = await fetch(`${ApiEndpoint.SupabaseSummaries}?range=${$selectedRange}`).then(
+      (response) => response.json(),
+    )
Evidence
The page now fetches summaries via fetch(...), but the developer documentation continues to
describe axios-based behavior, creating a mismatch between docs and reality.

CLAUDE.md[100-124]
src/routes/+page.svelte[32-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Developer docs reference axios even though axios was removed and the page now uses fetch.
### Issue Context
Keeping CLAUDE.md accurate matters because it’s used as the project’s operational/architecture guide.
### Fix Focus Areas
- CLAUDE.md[100-124]
### Suggested change
Replace axios-specific wording with fetch (or describe the current flow accurately), and remove the now-incorrect “axios used for internal API calls” note.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/routes/api/wakatime/current/durations/+server.ts Outdated
@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented May 2, 2026

Persistent review updated to latest commit 6e6435d

… in finally

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cesarnml cesarnml deleted the branch agents/p1-03-eslint-plugin-svelte3-eslint-plugin-svelte-full-migration May 2, 2026 10:14
@cesarnml cesarnml closed this May 2, 2026
@cesarnml cesarnml deleted the agents/p1-04-replace-axios-with-native-fetch branch May 2, 2026 10:14
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