fix(ui): show specific Bible passage error messages

[davidfedor]

Map BibleTextView errors to clearer user-facing messages for missing passages, missing or invalid app keys, forbidden access, rate limits, unreachable server/timeouts, and 5xx responses.

Extract the error-message mapping into a dedicated UI helper and extend component tests to cover the new cases.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 70bf233

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[greptile-apps]

Greptile Summary

This PR extracts Bible text error handling into a dedicated getBibleTextErrorMessage helper and wires it into BibleTextView, replacing the single hard-coded unavailable-verse string with seven distinct, context-aware messages (401, 403, 404, 429, 5xx, offline/network, and generic fallback).

Key changes:

• New packages/ui/src/lib/bible-text-error.ts utility with correct guard ordering: 5xx is evaluated before the 404/message.includes('not found') check, preventing the previously flagged shadowing bug.
• VerseUnavailableMessage refactored to accept a message prop; getBibleTextErrorMessage called at the BibleTextView error branch.
• Eight new test cases cover every mapped status code; the offline test correctly uses a neutral error message ('Unexpected connection state') so it only exercises the navigator.onLine guard, not the string-match path.
• Storybook Error story updated to expect SERVER_ERROR_MESSAGE (consistent with its MSW 500 mock).

One minor open point:

• The message.includes('not found') arm (line 47) runs for any error without a recognised status, not just those where status is explicitly undefined. Restricting it to status === undefined would prevent non-HTTP JS errors that happen to contain "not found" from being misclassified as passage-not-found errors.

Confidence Score: 5/5

Safe to merge — all previously flagged issues are resolved and the only remaining finding is a minor defensive hardening suggestion.

Both P1 issues from prior review rounds (5xx/not-found ordering and offline test isolation) are fully addressed. The single new comment is P2: the message.includes('not found') fallback could theoretically misclassify a non-HTTP JS error, but it will never cause data loss or a crash. All happy and error paths are covered by tests.

packages/ui/src/lib/bible-text-error.ts — the message.includes('not found') arm on line 47 could be tightened to only apply when status is undefined.

Important Files Changed

Filename	Overview
packages/ui/src/lib/bible-text-error.ts	New utility that maps BibleTextError objects to user-facing strings; 5xx guard is correctly ordered before the 404/"not found" check, but the message-only "not found" fallback (no status) remains a minor ambiguity risk.
packages/ui/src/components/verse.tsx	VerseUnavailableMessage now accepts a message prop and delegates to getBibleTextErrorMessage; old hard-coded constant removed cleanly.
packages/ui/src/components/verse.test.tsx	Eight new test cases cover all mapped status codes and the offline/unreachable-server paths; offline test now uses a neutral message that doesn't alias the isUnreachableServerError path.
packages/ui/src/components/bible-card.stories.tsx	Story's expected error text updated to SERVER_ERROR_MESSAGE to match the MSW 500 mock response; correct and consistent with new logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[BibleTextView receives error] --> B{error.status?}
    B -->|401| C[INVALID_APP_KEY_MESSAGE]
    B -->|403| D[FORBIDDEN_MESSAGE]
    B -->|429| E[RATE_LIMITED_MESSAGE]
    B -->|>= 500| F[SERVER_ERROR_MESSAGE]
    B -->|404| G[PASSAGE_NOT_FOUND_MESSAGE]
    B -->|undefined / other| H{message includes 'not found'?}
    H -->|yes| G
    H -->|no| I{navigator.onLine === false?}
    I -->|yes| J[UNREACHABLE_SERVER_MESSAGE]
    I -->|no| K{isUnreachableServerError message match?}
    K -->|yes| J
    K -->|no| L[GENERIC_ERROR_MESSAGE]

Loading

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into df/error-msgs" | Re-trigger Greptile}

[davidfedor]

@greptile are all your concerns addressed satisfactorily?

[greptile-apps]

Yes, both concerns are addressed:

P1 (5xx shadowing): Fixed by checking status >= 500 before the message.includes('not found') check. A 503 with "not found" in the message now correctly returns the server error message, as verified by the new test.

P2 (offline test isolation): Fixed by using 'Unexpected connection state' as the error message, which doesn't match any of the string patterns in isUnreachableServerError. The test now properly isolates the navigator.onLine === false code path.

[davidfedor]

@cameronapak I'd love your thoughts on this. Is there a different way you'd suggest that this be structured? Feel free to completely rewrite this if I'm not approaching it the right way.

[cameronapak]

This looks good to me. Thank you for making it where we have better, more specific error copy! Can you run pnpm changeset at the project's root to bump all of the packages, so that the changes propogate into the next version? (we bump them all at once for them to stay on the same version across the board for react sdk packages)