Fix: first-load error state for failed fetches

[0x3mr]

Fixes #76

• Board now shows "NO DATA — Real-time data unavailable" instead of spinning on "CONNECTING" indefinitely when every stop request fails
• Board recovers automatically once any request succeeds
• fetchFailed prop added to Board to carry the failure signal from page to component
• 'error' copy added to empty-board.svelte using the same red accent as the stale state
• Test added for the new NO DATA state; emptyMode comment updated to reflect the new branch

Results:

Summary by CodeRabbit

Summary by CodeRabbit

• New Features

  • Added an error-style “NO DATA / SIGNAL LOST” empty view for boards when the initial fetch fails.

• Bug Fixes

  • Improved board empty-state logic to distinguish connecting, stale, empty, and failed-load scenarios.
  • Ensured the “NO DATA” message shows correctly when the first load returns no results.

• Tests

  • Added coverage for the initial-load failure case to confirm the correct error empty view is rendered.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f27ddfe-254f-447c-8337-4976bc37e70f

📥 Commits

Reviewing files that changed from the base of the PR and between da8befa and 4e1af1d.

📒 Files selected for processing (2)

• src/components/board/board.svelte
• src/components/board/board.test.js

✅ Files skipped from review due to trivial changes (1)

• src/components/board/board.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

• src/components/board/board.svelte

---

📝 Walkthrough

Walkthrough

This PR adds fetchFailed tracking on the stop page, passes it into Board, adds a new 'error' empty mode and matching copy, and updates tests for the first-load failure case.

Changes

First-load Error State

Layer / File(s)	Summary
Page-level fetch failure tracking src/routes/stops/[stopID]/+page.svelte	Tracks fetchFailed, sets it on initial-load failure, resets it on success, and passes it to Board.
Board error mode and copy src/components/board/board.svelte, src/components/board/empty-board.svelte	Adds fetchFailed handling, derives 'error' when no data exists yet, and adds matching EMPTY copy for the error state.
Tests for error empty state src/components/board/board.test.js	Updates the branching note and adds assertions covering first-load failure and the non-first-load case.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Page as +page.svelte
  participant Board as board.svelte
  participant EmptyBoard as empty-board.svelte

  Page->>Page: fetchAll() returns no fulfilled results
  alt lastUpdatedAt === null
    Page->>Page: set fetchFailed = true
  else lastUpdatedAt exists
    Page->>Page: keep existing behavior
  end
  Page->>Board: pass fetchFailed prop
  Board->>Board: derive emptyMode
  Board->>EmptyBoard: render error copy when mode='error'

Loading

Possibly related PRs

• OneBusAway/waystation#72: Shares the same board rendering and empty-state flow that this PR extends with a fetch-failure branch.
• OneBusAway/waystation#80: Also updates board.svelte empty-state behavior and empty-board.svelte copy in the same area.

🚥 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 summarizes the main change: first-load error handling for failed fetches.
Linked Issues check	✅ Passed	The change adds first-load fetch failure handling, shows a no-data state when lastUpdatedAt is null, and recovers after success.
Out of Scope Changes check	✅ Passed	All edits support the first-load failure UI, copy, test, and prop wiring; no unrelated changes are indicated.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[aaronbrethorst]

Hi Amr — this is a tidy, well-aimed fix. Issue #76's core complaint (the board spinning on "CONNECTING" forever when every stop request fails on first load) is genuinely solved: a first-load total failure now flips to a clear "SIGNAL LOST / NO DATA" state, and the moment any fetch succeeds you reset fetchFailed and the board recovers. The new emptyMode ternary ordering is correct, the empty-board.svelte copy slots cleanly into the existing state table, and the doc comment on the test was updated to keep the "easy-to-invert ordering" rationale honest. Tests (45) and lint both pass locally.

There's one thing I'd like pinned before merge, plus a simplification worth taking.

Critical Issues (0 found)

None. The shipped behavior is correct.

Important Issues (1 found)

• The !updatedDate && guard is untested — the one invariant that matters most has no net. [src/components/board/board.svelte:38-40, src/components/board/board.test.js:27-31] The whole point of !updatedDate && fetchFailed ? 'error' is that a failure flag must be suppressed once real data has been shown — a rider looking at last-known departures should see STALE, not "NO DATA / SIGNAL LOST". But the only test exercising fetchFailed: true also passes lastUpdatedAt: null, so you can delete the !updatedDate && guard entirely and all 45 tests still pass. That's a hole precisely where the interesting logic lives. Please add the mirror case in the same file using the existing renderEmpty helper:

  test('does not show NO DATA when a fetch fails but data already exists', () => {
      const { container } = renderEmpty({ lastUpdatedAt: now.getTime(), fetchFailed: true });
      expect(container.innerHTML).not.toContain('NO DATA');
  });

  One line of coverage, and it protects the exact behavior that distinguishes the error state from stale.

Suggestions (1 found)

• Reformulate emptyMode with $derived.by to match house style. [src/components/board/board.svelte:38-40] The 4-way chained ternary repeats !updatedDate and flattens two independent axes (do-we-have-data? / which-mode?) into one left-to-right chain. This directory already favors $derived.by(() => { … }) with early returns for multi-branch logic — see arrival-hero.svelte:11 and status-pip.svelte:14. An exactly-equivalent form:

  const emptyMode = $derived.by(() => {
      if (updatedDate) return stale ? 'stale' : 'empty';
      return fetchFailed ? 'error' : 'connecting';
  });

  With data → stale/empty; without → error/connecting. Each remaining ternary is a simple binary. Optional, but it reads more clearly and matches the neighbors.

Out of Scope (noted, not blocking)

These are pre-existing behaviors this PR does not introduce or worsen — flagging them so they don't get lost, not asking you to expand this PR:

• Partial failures are invisible. In a multi-stop board (+-separated URL), if some stops succeed and some fail, fulfilled.length > 0, so the failed stops are only console.error'd and the board renders as healthy. A rider can't tell "stop B has no service" from "stop B is broken." Worth a dedicated issue about surfacing per-stop failures — but it's orthogonal to the all-fail case you're fixing here.
• fetchAll has try/finally with no catch. A throw during the success-path processing (e.g. a malformed departure) on first load would still leave the board on "CONNECTING", since fetchFailed is only set in the all-rejected branch. Same failure class #76 is about, different code path. Also pre-existing; a follow-up could set fetchFailed from a real catch.

I did check the empty/malformed stopIDs edge case — since params.stopID.split('+') always yields at least one element, it isn't practically reachable, so I'm not counting it against you.

Strengths

• Clean recovery semantics: fetchFailed resets on every success and the error mode is gated on !updatedDate, so the state can never get stuck true after a recovery.
• Scoped the flag correctly — set only when lastUpdatedAt === null, letting the stale path own the "was working, now failing" case.
• The presentational branch matrix (connecting / empty / stale / error) is now fully enumerated, and the new state follows the existing DAMP test style.

Recommended Action

1. Add the guard test above (required — cheap, closes the real gap).
2. Take the $derived.by simplification (recommended).
3. Consider filing a follow-up issue for partial-failure surfacing.
4. Re-run after the test lands.

Verdict: Request Changes — solely to pin the !updatedDate && guard. Everything else here is solid.

[0x3mr]

3. Consider filing a follow-up issue for partial-failure surfacing.

Yes, I have that in plan #75, I'll get it done asap