From 76434c68c76db1f117965c4b4c9863087d196c27 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 18:37:58 +0000 Subject: [PATCH] Say what "No pod URL found" actually means, and stop guessing wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sentry has been collecting `Error: No pod URL found` from the packing list view. It comes from usePodSync: `getPrimaryPodUrl` returned null, so the hook threw a bare Error, view-packing-list's onSaveError handed the *string* to reportError, and the user was shown "Failed to save to Pod: No pod URL found". Three things were wrong with that. The null meant three different things. A signed-out session, a WebID profile we couldn't read this second, and an account that declares no storage at all are not the same problem — the first two are a moment, the third is a fact. `resolvePodUrl` now returns the reason alongside the URL, and `PodUrlUnavailableError` carries it to the callbacks. `getPrimaryPodUrl` keeps its old shape for the call sites that only want the URL. A momentary failure was the loudest thing in the app. The list view polls every five seconds, so a bad minute of network meant an error per tick; the save path turned the same condition into an exception in Sentry with only errorReporting's own frames for a stack, because it was given a string. Sync now skips a Pod it can't locate yet and lets the next poll pick it up, saves report through onSaveError with the error itself attached, and the page keeps a retryable one out of Sentry while still telling the user, in words, that the change is on this device only. An account with no storage declared reports as it always did — no later attempt fixes that one. And the lookup gave up early. A WebID profile is public by design, so when the authenticated read fails the token is the likeliest culprit: expired between check and request, a DPoP nonce race, a refresh still in flight. Retrying the same document unauthenticated costs one request in the only case where we would otherwise have given up — and giving up here does not just skip a poll, it drops a save. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KeURbAVUoDvihv7cvqDkLE --- src/hooks/usePodSync.test.ts | 119 ++++++++++++++++++++++----- src/hooks/usePodSync.ts | 55 ++++++++----- src/pages/sharing-settings.test.tsx | 3 + src/pages/view-packing-list.test.tsx | 88 ++++++++++++++++++++ src/pages/view-packing-list.tsx | 16 +++- src/services/solidPod.test.ts | 96 +++++++++++++++++++++ src/services/solidPod.ts | 117 +++++++++++++++++++++----- 7 files changed, 430 insertions(+), 64 deletions(-) diff --git a/src/hooks/usePodSync.test.ts b/src/hooks/usePodSync.test.ts index d7bbba97..6d947372 100644 --- a/src/hooks/usePodSync.test.ts +++ b/src/hooks/usePodSync.test.ts @@ -9,24 +9,35 @@ vi.mock('../components/SolidPodContext', () => ({ useSolidPod: vi.fn(), })) -vi.mock('../services/solidPod', () => ({ - getPrimaryPodUrl: vi.fn(), - loadRdfFromPod: vi.fn(), - saveRdfToPod: vi.fn(), - AuthenticationError: class AuthenticationError extends Error { - constructor(message: string) { - super(message) - this.name = 'AuthenticationError' +vi.mock('../services/solidPod', () => { + class PodUrlUnavailableError extends Error { + constructor(public readonly reason: string) { + super(`pod url unavailable: ${reason}`) + this.name = 'PodUrlUnavailableError' } - }, -})) + } + return { + resolvePodUrl: vi.fn(), + loadRdfFromPod: vi.fn(), + saveRdfToPod: vi.fn(), + PodUrlUnavailableError, + isRetryablePodUrlFailure: (error: unknown) => + error instanceof PodUrlUnavailableError && error.reason !== 'no-storage-declared', + AuthenticationError: class AuthenticationError extends Error { + constructor(message: string) { + super(message) + this.name = 'AuthenticationError' + } + }, + } +}) import { useSolidPod } from '../components/SolidPodContext' -import { getPrimaryPodUrl, loadRdfFromPod, saveRdfToPod } from '../services/solidPod' +import { resolvePodUrl, loadRdfFromPod, saveRdfToPod } from '../services/solidPod' import type { AppSession as Session } from '../../types/AppSession' const mockUseSolidPod = vi.mocked(useSolidPod) -const mockGetPrimaryPodUrl = vi.mocked(getPrimaryPodUrl) +const mockResolvePodUrl = vi.mocked(resolvePodUrl) const mockLoadRdfFromPod = vi.mocked(loadRdfFromPod) const mockSaveRdfToPod = vi.mocked(saveRdfToPod) @@ -51,7 +62,7 @@ function setupLoggedIn() { login: vi.fn(), logout: vi.fn(), }) - mockGetPrimaryPodUrl.mockResolvedValue(POD_URL) + mockResolvePodUrl.mockResolvedValue({ podUrl: POD_URL }) mockLoadRdfFromPod.mockResolvedValue(QUESTION_SET_DATA) } @@ -77,7 +88,7 @@ describe('usePodSync', () => { beforeEach(() => { vi.useFakeTimers() vi.spyOn(console, 'error').mockImplementation(() => {}) - mockGetPrimaryPodUrl.mockReset() + mockResolvePodUrl.mockReset() mockLoadRdfFromPod.mockReset() mockSaveRdfToPod.mockReset() }) @@ -360,7 +371,7 @@ describe('usePodSync', () => { }) expect(success).toBe(false) - expect(onSaveError).toHaveBeenCalledWith('network error') + expect(onSaveError).toHaveBeenCalledWith('network error', expect.any(Error)) }) it('does not save when not logged in and no foreign pod configured', async () => { @@ -459,14 +470,78 @@ describe('usePodSync', () => { await result.current.syncFromPod() }) - expect(onSyncError).toHaveBeenCalledWith(expect.stringContaining('Not Found')) + expect(onSyncError).toHaveBeenCalledWith(expect.stringContaining('Not Found'), expect.any(Error)) + }) + }) + + describe('unknown Pod location', () => { + it('does not report a sync when the Pod location is only temporarily unknown', async () => { + // The poll runs every few seconds; reporting a bad minute of network would + // mean an error per tick for something the next tick fixes. + setupLoggedIn() + mockResolvePodUrl.mockResolvedValue({ podUrl: null, reason: 'profile-unreachable' }) + const onSyncError = vi.fn() + + const { result } = renderHook(() => + usePodSync({ pathConfig: staticPathConfig, enabled: true, onSyncError, rdf: rdfOptions }) + ) + + await act(async () => { + await result.current.syncFromPod() + }) + + expect(onSyncError).not.toHaveBeenCalled() + expect(mockLoadRdfFromPod).not.toHaveBeenCalled() + }) + + it('reports a sync when the account declares no storage at all', async () => { + // Nothing about this one gets better on the next poll. + setupLoggedIn() + mockResolvePodUrl.mockResolvedValue({ podUrl: null, reason: 'no-storage-declared' }) + const onSyncError = vi.fn() + + const { result } = renderHook(() => + usePodSync({ pathConfig: staticPathConfig, enabled: true, onSyncError, rdf: rdfOptions }) + ) + + await act(async () => { + await result.current.syncFromPod() + }) + + expect(onSyncError).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ name: 'PodUrlUnavailableError', reason: 'no-storage-declared' }) + ) + }) + + it('fails a save loudly, and hands the reason to onSaveError', async () => { + // A dropped write is never silent — the local copy is the only one left. + setupLoggedIn() + mockResolvePodUrl.mockResolvedValue({ podUrl: null, reason: 'profile-unreachable' }) + const onSaveError = vi.fn() + + const { result } = renderHook(() => + usePodSync({ pathConfig: staticPathConfig, enabled: true, onSaveError, rdf: rdfOptions }) + ) + + let success: boolean | undefined + await act(async () => { + success = await result.current.saveToPod(QUESTION_SET_DATA) + }) + + expect(success).toBe(false) + expect(mockSaveRdfToPod).not.toHaveBeenCalled() + expect(onSaveError).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ name: 'PodUrlUnavailableError', reason: 'profile-unreachable' }) + ) }) }) describe('podUrl override in pathConfig', () => { const FOREIGN_POD_URL = 'https://alice.solidcommunity.net/' - it('syncFromPod uses pathConfig.podUrl instead of getPrimaryPodUrl when provided', async () => { + it('syncFromPod uses pathConfig.podUrl instead of resolving the signed-in Pod', async () => { setupLoggedIn() const { result } = renderHook(() => @@ -486,7 +561,7 @@ describe('usePodSync', () => { await result.current.syncFromPod() }) - expect(mockGetPrimaryPodUrl).not.toHaveBeenCalled() + expect(mockResolvePodUrl).not.toHaveBeenCalled() expect(mockLoadRdfFromPod).toHaveBeenCalledWith( mockSession, `${FOREIGN_POD_URL}pack-me-up/packing-lists/list-abc.ttl`, @@ -494,7 +569,7 @@ describe('usePodSync', () => { ) }) - it('saveToPod uses pathConfig.podUrl instead of getPrimaryPodUrl when provided', async () => { + it('saveToPod uses pathConfig.podUrl instead of resolving the signed-in Pod', async () => { setupLoggedIn() const { result } = renderHook(() => @@ -514,7 +589,7 @@ describe('usePodSync', () => { await result.current.saveToPod({ id: 'list-abc', name: 'Test' }) }) - expect(mockGetPrimaryPodUrl).not.toHaveBeenCalled() + expect(mockResolvePodUrl).not.toHaveBeenCalled() expect(mockSaveRdfToPod).toHaveBeenCalledWith( expect.objectContaining({ fileUrl: `${FOREIGN_POD_URL}pack-me-up/packing-lists/list-abc.ttl`, @@ -522,7 +597,7 @@ describe('usePodSync', () => { ) }) - it('falls back to getPrimaryPodUrl when pathConfig.podUrl is absent', async () => { + it('resolves the signed-in Pod when pathConfig.podUrl is absent', async () => { setupLoggedIn() const { result } = renderHook(() => @@ -536,7 +611,7 @@ describe('usePodSync', () => { await result.current.syncFromPod() }) - expect(mockGetPrimaryPodUrl).toHaveBeenCalled() + expect(mockResolvePodUrl).toHaveBeenCalled() }) }) }) diff --git a/src/hooks/usePodSync.ts b/src/hooks/usePodSync.ts index d4e7fc85..0fc5c497 100644 --- a/src/hooks/usePodSync.ts +++ b/src/hooks/usePodSync.ts @@ -1,7 +1,8 @@ import { useState, useEffect, useCallback, useRef, useMemo } from 'react'; import type { SolidDataset } from '@inrupt/solid-client'; import { useSolidPod } from '../components/SolidPodContext'; -import { getPrimaryPodUrl, loadRdfFromPod, saveRdfToPod, AuthenticationError } from '../services/solidPod'; +import { resolvePodUrl, loadRdfFromPod, saveRdfToPod, AuthenticationError, PodUrlUnavailableError, isRetryablePodUrlFailure } from '../services/solidPod'; +import type { AppSession } from '../types/AppSession'; import { profile, profileEvent } from '../utils/profiling'; /** @@ -26,7 +27,7 @@ export interface PodPathConfig { resourceId?: string | null; /** - * Override the pod URL — bypasses getPrimaryPodUrl when set. + * Override the pod URL — bypasses resolvePodUrl when set. * Use this to sync with a foreign user's pod (e.g. a shared list). */ podUrl?: string; @@ -63,9 +64,11 @@ export interface PodSyncOptions { onSyncSuccess?: (data: T) => void; /** - * Callback when sync from Pod fails + * Callback when sync from Pod fails. + * `cause` is the error itself, so callers can tell a Pod we couldn't reach + * this second from a real fault worth reporting. */ - onSyncError?: (error: string) => void; + onSyncError?: (error: string, cause?: unknown) => void; /** * Callback when save to Pod succeeds @@ -73,9 +76,10 @@ export interface PodSyncOptions { onSaveSuccess?: () => void; /** - * Callback when save to Pod fails + * Callback when save to Pod fails. + * `cause` is the error itself — see `onSyncError`. */ - onSaveError?: (error: string) => void; + onSaveError?: (error: string, cause?: unknown) => void; /** * Whether sync is enabled @@ -91,6 +95,20 @@ export interface PodSyncState { syncFromPod: () => Promise; } +/** + * Where this user's Pod lives, or a `PodUrlUnavailableError` saying why we don't + * know. Throwing rather than returning null keeps the reason attached all the + * way to the callbacks, which is what lets a momentary blip stay out of the + * user's face and out of Sentry. + */ +async function requirePodUrl(session: AppSession | null): Promise { + const { podUrl, reason } = await resolvePodUrl(session); + if (!podUrl) { + throw new PodUrlUnavailableError(reason ?? 'no-session'); + } + return podUrl; +} + /** * Generic hook for automatic synchronization with Solid Pod * Polls the pod at regular intervals and provides manual sync functions @@ -219,11 +237,7 @@ export function usePodSync(options: PodSyncOptions): PodSyncState { setError(null); try { - const podUrl = pathConfigRef.current.podUrl ?? await getPrimaryPodUrl(session); - - if (!podUrl) { - throw new Error('No pod URL found'); - } + const podUrl = pathConfigRef.current.podUrl ?? await requirePodUrl(session); const fileUrl = getFileUrl(podUrl); @@ -244,7 +258,14 @@ export function usePodSync(options: PodSyncOptions): PodSyncState { // 404 on own pod = file not yet created (expected) → silent // 404 on foreign pod (pathConfig.podUrl set) = file missing or access denied → report const isSilentMiss = statusCode === 404 && !pathConfigRef.current.podUrl - if (!isSilentMiss) { + // Not knowing where the Pod is yet is not a failed sync, it is a sync that + // hasn't started. This poll runs every few seconds, so the next one picks + // it up — reporting it would mean an error per tick for a bad minute of + // network. An account that declares no storage at all is not retryable and + // still reports. + if (isRetryablePodUrlFailure(err)) { + console.warn('Skipping sync from Pod:', (err as Error).message) + } else if (!isSilentMiss) { // Authentication errors use their own message const errorMessage = err instanceof AuthenticationError ? err.message @@ -252,7 +273,7 @@ export function usePodSync(options: PodSyncOptions): PodSyncState { setError(errorMessage); if (onSyncErrorRef.current) { - onSyncErrorRef.current(errorMessage); + onSyncErrorRef.current(errorMessage, err); } } } finally { @@ -273,11 +294,7 @@ export function usePodSync(options: PodSyncOptions): PodSyncState { profileEvent('podSync.saveToPod.start'); try { - const podUrl = pathConfigRef.current.podUrl ?? await getPrimaryPodUrl(session!); - - if (!podUrl) { - throw new Error('No pod URL found'); - } + const podUrl = pathConfigRef.current.podUrl ?? await requirePodUrl(session); const fileUrl = getFileUrl(podUrl); @@ -307,7 +324,7 @@ export function usePodSync(options: PodSyncOptions): PodSyncState { setError(errorMessage); if (onSaveErrorRef.current) { - onSaveErrorRef.current(errorMessage); + onSaveErrorRef.current(errorMessage, err); } return false; diff --git a/src/pages/sharing-settings.test.tsx b/src/pages/sharing-settings.test.tsx index 810e2833..acdba714 100644 --- a/src/pages/sharing-settings.test.tsx +++ b/src/pages/sharing-settings.test.tsx @@ -15,6 +15,9 @@ vi.mock('../services/solidPod', () => ({ getCollaborators: vi.fn(() => Promise.resolve([])), isPubliclyAccessible: vi.fn(() => Promise.resolve(false)), getPrimaryPodUrl: vi.fn(() => Promise.resolve('https://pod.example.com/')), + resolvePodUrl: vi.fn(() => Promise.resolve({ podUrl: 'https://pod.example.com/' })), + isRetryablePodUrlFailure: () => false, + PodUrlUnavailableError: class PodUrlUnavailableError extends Error {}, getPodOwnerName: vi.fn(() => Promise.resolve(null)), friendlyPodName: vi.fn((url: string) => url), resolveOwnerDisplayName: vi.fn((foafName: string | null | undefined, ownerWebId: string | null | undefined, podUrl: string) => foafName ?? ownerWebId ?? podUrl), diff --git a/src/pages/view-packing-list.test.tsx b/src/pages/view-packing-list.test.tsx index 16d234e0..f7149ddf 100644 --- a/src/pages/view-packing-list.test.tsx +++ b/src/pages/view-packing-list.test.tsx @@ -45,8 +45,12 @@ vi.mock('../services/solidPod', () => ({ resolveOwnerDisplayName: vi.fn((foafName: string | null | undefined, ownerWebId: string | null | undefined, podUrl: string) => foafName ?? ownerWebId ?? podUrl), getPodOwnerName: vi.fn().mockResolvedValue(null), deriveWebIdFromPodUrl: vi.fn((url: string) => `${url.replace(/\/+$/, '')}/profile/card#me`), + isRetryablePodUrlFailure: (error: unknown) => + error instanceof Error && error.name === 'PodUrlUnavailableError' + && (error as { reason?: string }).reason !== 'no-storage-declared', })) + vi.mock('../components/SharePackingListModal', () => ({ SharePackingListModal: vi.fn(() => null), })) @@ -4053,3 +4057,87 @@ describe('a section named after a question', () => { await waitFor(() => expect(screen.getAllByTestId('list-section').length).toBe(1)) }) }) + + +// ─── Reporting a save that never reached the Pod ───────────────────────────── + +describe('ViewPackingList save-to-Pod failures', () => { + class PodUrlUnavailableError extends Error { + constructor(public readonly reason: string) { + super(reason === 'no-storage-declared' ? 'No pod found for your account' : "Couldn't reach your Pod. This change is saved on this device only.") + this.name = 'PodUrlUnavailableError' + } + } + + beforeEach(() => { + mockShowToast.mockClear() + mockCaptureException.mockClear() + vi.spyOn(console, 'warn').mockImplementation(() => {}) + vi.spyOn(console, 'error').mockImplementation(() => {}) + mockUseSolidPod.mockReturnValue({ + isLoggedIn: false, + session: null, + webId: undefined, + isLoading: false, + login: vi.fn(), + logout: vi.fn(), + }) + mockUsePodSync.mockReturnValue({ saveToPod: vi.fn() }) + mockUseSyncCoordinator.mockReturnValue({ + syncingFromPod: false, + handleSyncSuccess: vi.fn(), + handleSyncError: vi.fn(), + saveWithSyncPrevention: vi.fn().mockResolvedValue({ ...testPackingList, _rev: '2' }), + }) + mockUseDatabase.mockReturnValue({ db: makeDb() as unknown as PackingAppDatabase }) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + /** The onSaveError the page handed to usePodSync. */ + async function saveErrorHandler() { + renderComponent() + await waitFor(() => expect(mockUsePodSync).toHaveBeenCalled()) + const options = mockUsePodSync.mock.calls.at(-1)![0] as { onSaveError: (message: string, cause?: unknown) => void } + return options.onSaveError + } + + it('keeps a Pod we could not reach out of Sentry, and says what happened', async () => { + // This is the "No pod URL found" issue: a few seconds of bad network + // reported as an exception, and shown to the user as an internal string. + const onSaveError = await saveErrorHandler() + const cause = new PodUrlUnavailableError('profile-unreachable') + + act(() => onSaveError(cause.message, cause)) + + expect(mockCaptureException).not.toHaveBeenCalled() + expect(mockShowToast).toHaveBeenCalledWith(cause.message, 'error') + }) + + it('still reports a save that failed for any other reason', async () => { + const onSaveError = await saveErrorHandler() + const cause = new Error('500 Internal Server Error') + + act(() => onSaveError(cause.message, cause)) + + // The error itself, not its message: a string reaches Sentry with only + // errorReporting's own frames for a stack. + expect(mockCaptureException).toHaveBeenCalledWith(cause) + expect(mockShowToast).toHaveBeenCalledWith( + 'Failed to save to Pod: 500 Internal Server Error', + 'error', + expect.stringContaining('500 Internal Server Error') + ) + }) + + it('reports an account with no Pod, which no later save will fix', async () => { + const onSaveError = await saveErrorHandler() + const cause = new PodUrlUnavailableError('no-storage-declared') + + act(() => onSaveError(cause.message, cause)) + + expect(mockCaptureException).toHaveBeenCalledWith(cause) + }) +}) diff --git a/src/pages/view-packing-list.tsx b/src/pages/view-packing-list.tsx index 3cfaff5d..72148056 100644 --- a/src/pages/view-packing-list.tsx +++ b/src/pages/view-packing-list.tsx @@ -14,7 +14,7 @@ import { reportError } from '../errorReporting' import { usePodSync } from '../hooks/usePodSync' import { useLocalFirstLoad } from '../hooks/useLocalFirstLoad' import { useSyncCoordinator } from '../hooks/useSyncCoordinator' -import { POD_CONTAINERS, getPrimaryPodUrl, saveRdfToPod, resolveOwnerDisplayName, deriveWebIdFromPodUrl } from '../services/solidPod' +import { POD_CONTAINERS, getPrimaryPodUrl, saveRdfToPod, resolveOwnerDisplayName, deriveWebIdFromPodUrl, isRetryablePodUrlFailure } from '../services/solidPod' import { useOwnerDisplayName } from '../hooks/useOwnerDisplayName' import { packingListToDataset, datasetToPackingList } from '../services/rdfSerialization' import { SharePackingListModal } from '../components/SharePackingListModal' @@ -704,8 +704,18 @@ export function ViewPackingList() { }, []); // Callback when save to Pod fails - const handleSaveError = useCallback((error: string) => { - const details = reportError(error, 'Save to Pod error'); + const handleSaveError = useCallback((error: string, cause?: unknown) => { + // A Pod whose address we couldn't look up this second is a network + // condition, not a fault: the edit is already in the local database, and + // the next save carries it up. Say that in words the user can act on, and + // keep it out of Sentry — it arrived there as a bare "No pod URL found" + // with only errorReporting's own frames for a stack. + if (isRetryablePodUrlFailure(cause)) { + console.warn('Save to Pod skipped:', error); + showToast(error, 'error'); + return; + } + const details = reportError(cause ?? error, 'Save to Pod error'); showToast(`Failed to save to Pod: ${error}`, 'error', details); }, [showToast]); diff --git a/src/services/solidPod.test.ts b/src/services/solidPod.test.ts index 405a9a06..f8e0eaa7 100644 --- a/src/services/solidPod.test.ts +++ b/src/services/solidPod.test.ts @@ -25,6 +25,10 @@ import { derivePodUrlFromWebId, podUsernameFromWebId, resetPodSessionCaches, + resolvePodUrl, + isRetryablePodUrlFailure, + PodUrlUnavailableError, + POD_ERROR_MESSAGES, } from './solidPod' import { AuthenticationError } from './solidPod' import { PackingAppDatabase } from './database' @@ -1755,6 +1759,98 @@ describe('getPrimaryPodUrl', () => { expect(await getPrimaryPodUrl(sessionFor('https://id.inrupt.com/someoneelse'))).toBeNull() }) + + it('reads the profile unauthenticated when the authenticated read fails', async () => { + // A WebID profile is public by design, so a token problem is the likeliest + // reason the authenticated read failed — and a plain fetch sidesteps it. + mockGetPodUrlAll.mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), { statusCode: 401 })) + mockGetPodUrlAll.mockResolvedValueOnce([ESS_POD_URL]) + const session = sessionFor(ESS_WEB_ID) + + expect(await getPrimaryPodUrl(session)).toBe(ESS_POD_URL) + expect(mockGetPodUrlAll).toHaveBeenNthCalledWith(1, ESS_WEB_ID, expect.objectContaining({ fetch: session.fetch })) + expect(mockGetPodUrlAll).toHaveBeenNthCalledWith(2, ESS_WEB_ID) + }) + + it('caches an unauthenticated answer like any other, so it is read once', async () => { + mockGetPodUrlAll.mockRejectedValueOnce(new TypeError('Failed to fetch')) + mockGetPodUrlAll.mockResolvedValue([ESS_POD_URL]) + const session = sessionFor(ESS_WEB_ID) + + expect(await getPrimaryPodUrl(session)).toBe(ESS_POD_URL) + expect(await getPrimaryPodUrl(session)).toBe(ESS_POD_URL) + + expect(mockGetPodUrlAll).toHaveBeenCalledTimes(2) + }) + + it('gives up only when the unauthenticated read fails too', async () => { + mockGetPodUrlAll.mockRejectedValueOnce(new TypeError('Failed to fetch')) + mockGetPodUrlAll.mockRejectedValueOnce(new TypeError('Failed to fetch')) + + expect(await getPrimaryPodUrl(sessionFor(ESS_WEB_ID))).toBeNull() + }) +}) + +// ─── resolvePodUrl ────────────────────────────────────────────────────────── + +describe('resolvePodUrl', () => { + const ESS_WEB_ID = 'https://id.inrupt.com/hannahwprior' + const ESS_POD_URL = 'https://storage.inrupt.com/d8c8c02b-b47c-48e9-b737-619f2958689f/' + + const sessionFor = (webId: string) => ({ + info: { isLoggedIn: true, webId }, + fetch: vi.fn(), + } as unknown as Session) + + beforeEach(() => { + localStorage.clear() + vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('reports a signed-out session as no-session', async () => { + expect(await resolvePodUrl(null)).toEqual({ podUrl: null, reason: 'no-session' }) + }) + + it('reports an unreadable profile as profile-unreachable', async () => { + mockGetPodUrlAll.mockRejectedValue(new TypeError('Failed to fetch')) + + expect(await resolvePodUrl(sessionFor(ESS_WEB_ID))) + .toEqual({ podUrl: null, reason: 'profile-unreachable' }) + }) + + it('reports a readable profile that declares no storage as no-storage-declared', async () => { + mockGetPodUrlAll.mockResolvedValueOnce([]) + + expect(await resolvePodUrl(sessionFor(ESS_WEB_ID))) + .toEqual({ podUrl: null, reason: 'no-storage-declared' }) + }) + + it('reports no reason when the Pod URL is known', async () => { + mockGetPodUrlAll.mockResolvedValueOnce([ESS_POD_URL]) + + expect(await resolvePodUrl(sessionFor(ESS_WEB_ID))).toEqual({ podUrl: ESS_POD_URL }) + }) + + it('separates the failures a later attempt can fix from the ones it cannot', () => { + expect(isRetryablePodUrlFailure(new PodUrlUnavailableError('profile-unreachable'))).toBe(true) + expect(isRetryablePodUrlFailure(new PodUrlUnavailableError('no-session'))).toBe(true) + expect(isRetryablePodUrlFailure(new PodUrlUnavailableError('no-storage-declared'))).toBe(false) + expect(isRetryablePodUrlFailure(new Error('Failed to fetch'))).toBe(false) + expect(isRetryablePodUrlFailure('No pod URL found')).toBe(false) + }) + + it('gives each reason a message that says what actually happened', () => { + expect(new PodUrlUnavailableError('profile-unreachable').message) + .toBe(POD_ERROR_MESSAGES.POD_UNREACHABLE) + expect(new PodUrlUnavailableError('no-storage-declared').message) + .toBe(POD_ERROR_MESSAGES.NO_POD_FOUND) + expect(new PodUrlUnavailableError('no-session').message) + .toBe(POD_ERROR_MESSAGES.NOT_LOGGED_IN) + }) }) // ─── derivePodUrlFromWebId ─────────────────────────────────────────────────── diff --git a/src/services/solidPod.ts b/src/services/solidPod.ts index 2887ce43..1e36466c 100644 --- a/src/services/solidPod.ts +++ b/src/services/solidPod.ts @@ -35,6 +35,7 @@ export const POD_ERROR_MESSAGES = { NOT_LOGGED_IN: 'You must be logged in to save to Pod', NOT_LOGGED_IN_LOAD: 'You must be logged in to load from Pod', NO_POD_FOUND: 'No pod found for your account', + POD_UNREACHABLE: "Couldn't reach your Pod. This change is saved on this device only.", SAVE_FAILED: 'Failed to save to Pod. Please try again.', LOAD_FAILED: 'Failed to load from Pod. Please try again.', NO_DATA_FOUND: (resourceType: string) => `No ${resourceType} found in Pod`, @@ -611,17 +612,60 @@ function cachePodUrl(webId: string, podUrl: string): void { } /** - * Validates session and retrieves the user's primary Pod URL. + * Why the app could not work out where a user's Pod lives. + * + * The distinction is the whole point: `profile-unreachable` and `no-session` + * describe a moment, and the next attempt may well succeed, so they belong in + * the console rather than in the user's face or in Sentry. `no-storage-declared` + * is a settled fact about the account that will never fix itself. + */ +export type PodUrlUnavailableReason = 'no-session' | 'profile-unreachable' | 'no-storage-declared' + +const POD_URL_UNAVAILABLE_MESSAGES: Record = { + 'no-session': POD_ERROR_MESSAGES.NOT_LOGGED_IN, + 'profile-unreachable': POD_ERROR_MESSAGES.POD_UNREACHABLE, + 'no-storage-declared': POD_ERROR_MESSAGES.NO_POD_FOUND, +} + +/** + * Thrown when a Pod read or write cannot start because the Pod's location is + * unknown. It carries the reason so callers can tell a blip from a dead end — + * this used to be a bare `new Error('No pod URL found')`, which reached the user + * as "Failed to save to Pod: No pod URL found" and Sentry as an exception with + * no stack of its own, for what is usually a few seconds of bad network. + */ +export class PodUrlUnavailableError extends Error { + constructor(public readonly reason: PodUrlUnavailableReason) { + super(POD_URL_UNAVAILABLE_MESSAGES[reason]) + this.name = 'PodUrlUnavailableError' + } +} + +/** True for an unknown Pod location that a later attempt can still resolve. */ +export function isRetryablePodUrlFailure(error: unknown): boolean { + return error instanceof PodUrlUnavailableError && error.reason !== 'no-storage-declared' +} + +/** + * Where a user's Pod lives, or why we don't know. + */ +export interface PodUrlResolution { + podUrl: string | null + /** Set only when `podUrl` is null. */ + reason?: PodUrlUnavailableReason +} + +/** + * Validates session and retrieves the user's primary Pod URL, saying why when + * it can't. * * The `pim:storage` triple in the WebID profile is the only authoritative source * for where a Pod lives, so an unreadable profile means "unknown", never "guess * from the WebID host". - * - * @returns Pod URL if it can be determined, null otherwise */ -export async function getPrimaryPodUrl(session: Session | null): Promise { +export async function resolvePodUrl(session: Session | null): Promise { if (!session || !session.info.isLoggedIn || !session.info.webId) { - return null + return { podUrl: null, reason: 'no-session' } } const webId = session.info.webId @@ -633,24 +677,32 @@ export async function getPrimaryPodUrl(session: Session | null): Promise result.podUrl) - podUrlByWebId.set(webId, podUrl) + const attempt = resolvePrimaryPodUrl(session, webId) + const resolution = attempt.then(result => result.resolution) + podUrlByWebId.set(webId, resolution) // Only an answer actually read from the profile is worth keeping for the // session. A fallback — or a failure — says nothing about where the Pod is, // and caching it would leave the app stuck on one dropped request. - resolution.then( + attempt.then( result => { if (!result.authoritative) podUrlByWebId.delete(webId) }, () => podUrlByWebId.delete(webId) ) - return podUrl + return resolution +} + +/** + * The user's primary Pod URL, or null when it can't be determined. Callers that + * need to tell a blip from a dead end should use `resolvePodUrl` instead. + */ +export async function getPrimaryPodUrl(session: Session | null): Promise { + return (await resolvePodUrl(session)).podUrl } /** * The pod URL resolved for each WebID this session has asked about, including * requests still in flight. Cleared by `resetPodSessionCaches`. */ -const podUrlByWebId = new Map>() +const podUrlByWebId = new Map>() /** * Forget everything cached for the length of a session: which pod a WebID @@ -663,6 +715,26 @@ export function resetPodSessionCaches(): void { knownContainers.clear() } +/** + * The storage locations a WebID profile advertises. + * + * A WebID profile document is public by design — it is what an unauthenticated + * client reads to discover where a Pod lives — so when the *authenticated* read + * fails, the token is the likeliest culprit: an access token that expired + * between the check and the request, a DPoP nonce race, a refresh still in + * flight. A plain fetch of the same document sidesteps all of those, and costs + * one request in the only case where we would otherwise have given up. That + * matters because giving up here does not just skip a poll: it drops a save. + */ +async function readPodUrlsFromProfile(session: Session, webId: string): Promise { + try { + return await profile('pod.getPrimaryPodUrl', () => getPodUrlAll(webId, { fetch: session.fetch }), { webId }) + } catch (err) { + console.warn('getPrimaryPodUrl: authenticated profile read failed, retrying unauthenticated', err) + return await profile('pod.getPrimaryPodUrl.unauthenticated', () => getPodUrlAll(webId), { webId }) + } +} + /** * `authoritative` marks an answer that came from the profile itself, as opposed * to a last-known-good fallback — only the former is worth remembering. @@ -670,22 +742,22 @@ export function resetPodSessionCaches(): void { async function resolvePrimaryPodUrl( session: Session, webId: string -): Promise<{ podUrl: string | null; authoritative: boolean }> { +): Promise<{ resolution: PodUrlResolution; authoritative: boolean }> { let podUrls: string[] try { - podUrls = await profile('pod.getPrimaryPodUrl', () => getPodUrlAll(webId, { fetch: session.fetch }), { webId }) + podUrls = await readPodUrlsFromProfile(session, webId) } catch (err) { - // The profile document itself couldn't be fetched (transient network - // error, DPoP nonce race, expired token). We know nothing about the + // The profile document itself couldn't be fetched, signed in or not + // (transient network error, provider outage). We know nothing about the // Pod's location, so reuse the last known one and otherwise give up — - // callers report "no pod" and retry on the next sync. + // callers retry on the next sync. console.warn('getPrimaryPodUrl: could not read the WebID profile', err) - return { podUrl: readCachedPodUrl(webId), authoritative: false } + return { resolution: fallbackResolution(webId, 'profile-unreachable'), authoritative: false } } if (podUrls && podUrls.length > 0) { cachePodUrl(webId, podUrls[0]) - return { podUrl: podUrls[0], authoritative: true } + return { resolution: { podUrl: podUrls[0] }, authoritative: true } } // Profile was readable but declares no pim:storage — the case for CSS v7, @@ -693,10 +765,15 @@ async function resolvePrimaryPodUrl( const derivedPodUrl = derivePodUrlFromWebId(webId) if (derivedPodUrl) { cachePodUrl(webId, derivedPodUrl) - return { podUrl: derivedPodUrl, authoritative: true } + return { resolution: { podUrl: derivedPodUrl }, authoritative: true } } - return { podUrl: readCachedPodUrl(webId), authoritative: false } + return { resolution: fallbackResolution(webId, 'no-storage-declared'), authoritative: false } +} + +function fallbackResolution(webId: string, reason: PodUrlUnavailableReason): PodUrlResolution { + const cached = readCachedPodUrl(webId) + return cached ? { podUrl: cached } : { podUrl: null, reason } } /**