fix: preserve provider operation failures (#163)

.changelog/NEXT.md

@@ -50,6 +50,7 @@
 
 ## Fixed
 
+- **[issue-163] Provider operation failures stay actionable** — Browser connection and provider scraper failures now surface as recoverable errors instead of being reported as a logged-out session or an empty tree list.
 - **[issue-162] Provider refresh page cleanup** — Provider comparison refreshes now close their temporary browser page even when Ancestry navigation or scraping fails, preventing failed retries from accumulating pages in the shared browser.
 - Nominatim geocoding requests now time out after 15 seconds and are cancelled when their map-stream client disconnects, preventing stalled upstream sockets from blocking the shared geocoding queue.
 - AI discovery now rejects unsafe batch settings, limits background runs to one per family database, and lets an active run be cancelled without leaving provider work behind.

client/src/pages/GenealogyProviders.tsx

@@ -15,7 +15,8 @@ import {
   ToggleLeft,
   ToggleRight,
   ExternalLink,
-  Monitor
+  Monitor,
+  AlertCircle
 } from 'lucide-react';
 import toast from 'react-hot-toast';
 import { api, CredentialsStatus } from '../services/api';
@@ -50,6 +51,7 @@ export function GenealogyProvidersPage() {
   } = useBrowserConnection();
   const [providers, setProviders] = useState<ProviderInfo[]>([]);
   const [sessionStatus, setSessionStatus] = useState<Record<BuiltInProvider, ProviderSessionStatus>>({} as Record<BuiltInProvider, ProviderSessionStatus>);
+  const [sessionErrors, setSessionErrors] = useState<Partial<Record<BuiltInProvider, string>>>({});
   const [credentialsStatus, setCredentialsStatus] = useState<Record<BuiltInProvider, CredentialsStatus>>({} as Record<BuiltInProvider, CredentialsStatus>);
   const [loading, setLoading] = useState(true);
   const [checkingSession, setCheckingSession] = useState<BuiltInProvider | null>(null);
@@ -114,11 +116,22 @@ export function GenealogyProvidersPage() {
   const handleCheckSession = async (provider: BuiltInProvider) => {
     setCheckingSession(provider);
     const status = await api.checkProviderSession(provider).catch(err => {
+      setSessionErrors(prev => ({ ...prev, [provider]: err.message }));
+      setSessionStatus(prev => {
+        const next = { ...prev };
+        delete next[provider];
+        return next;
+      });
       toast.error(`Failed to check session: ${err.message}`);
       return null;
     });
 
     if (status) {
+      setSessionErrors(prev => {
+        const next = { ...prev };
+        delete next[provider];
+        return next;
+      });
       setSessionStatus(prev => ({ ...prev, [provider]: status }));
       if (status.loggedIn) {
         toast.success(`${provider}: Logged in${status.userName ? ` as ${status.userName}` : ''}`);
@@ -308,6 +321,7 @@ export function GenealogyProvidersPage() {
         {providers.map(({ provider, displayName, loginUrl }) => {
           const colors = providerColors[provider];
           const status = sessionStatus[provider];
+          const sessionError = sessionErrors[provider];
           const creds = credentialsStatus[provider];
           const isCheckingThis = checkingSession === provider;
           const isOpeningLoginThis = openingLogin === provider;
@@ -331,7 +345,12 @@ export function GenealogyProvidersPage() {
 
                     {/* Session Status */}
                     <div className="flex items-center gap-2">
-                      {status ? (
+                      {sessionError ? (
+                        <span className="flex items-center gap-1.5 text-app-warning">
+                          <AlertCircle size={18} />
+                          <span className="text-sm font-medium">Status unavailable</span>
+                        </span>
+                      ) : status ? (
                         status.loggedIn ? (
                           <span className="flex items-center gap-1.5 text-app-success">
                             <CheckCircle2 size={18} />
@@ -368,6 +387,29 @@ export function GenealogyProvidersPage() {
                 </div>
               </div>
 
+              {sessionError && (
+                <div className="mx-3 sm:mx-4 md:mx-5 mt-3 rounded-lg border border-app-warning/30 bg-app-warning-subtle p-3">
+                  <p className="text-sm text-app-warning">
+                    Could not verify this provider: {sessionError}
+                  </p>
+                  <div className="mt-2 flex flex-wrap items-center gap-3 text-xs">
+                    <Link
+                      to="/settings/browser"
+                      className="font-medium text-app-accent hover:underline"
+                    >
+                      Check browser settings
+                    </Link>
+                    <button
+                      onClick={() => handleCheckSession(provider)}
+                      disabled={isCheckingThis}
+                      className="font-medium text-app-accent hover:underline disabled:opacity-50"
+                    >
+                      Retry session check
+                    </button>
+                  </div>
+                </div>
+              )}
+
               {/* Login Options */}
               <div className="p-3 sm:p-4 md:p-5 space-y-4">
                 {/* Login Buttons */}

client/src/services/api.ts

@@ -24,6 +24,7 @@ import type {
   ExpandAncestryRequest,
   BuiltInProvider,
   ProviderSessionStatus,
+  ProviderTreeInfo,
   UserProviderConfig,
   ProviderComparison,
   ScrapedPersonData,
@@ -598,7 +599,7 @@ export const api = {
     }),
 
   listProviderTrees: (provider: BuiltInProvider) =>
-    fetchJson<Array<{ provider: BuiltInProvider; treeId: string; treeName: string }>>(`/scrape-providers/${provider}/trees`),
+    fetchJson<ProviderTreeInfo[]>(`/scrape-providers/${provider}/trees`),
 
   setProviderDefaultTree: (provider: BuiltInProvider, treeId?: string) =>
     fetchJson<UserProviderConfig>(`/scrape-providers/${provider}/default-tree`, {

server/src/routes/provider.routes.ts

@@ -108,24 +108,35 @@ router.post('/:provider/confirm-browser-login', (req: Request, res: Response) =>
 router.post('/:provider/check-session', async (req: Request, res: Response) => {
   const { provider } = req.params as { provider: BuiltInProvider };
 
-  const status = await providerService.checkSession(provider)
-    .catch(err => ({
-      provider,
-      enabled: false,
-      loggedIn: false,
-      lastChecked: new Date().toISOString(),
-      error: err.message
-    }));
-
-  res.json({ success: true, data: status });
+  const result = await providerService.checkSession(provider);
+  if (!result.success) {
+    res.status(503).json({
+      success: false,
+      error: result.error.message,
+      details: result.error
+    });
+    return;
+  }
+
+  res.json({ success: true, data: result.data });
 });
 
 /**
  * Check sessions for all enabled providers
  */
 router.post('/check-all-sessions', asyncHandler(async (_req: Request, res: Response) => {
-  const statuses = await providerService.checkAllSessions();
-  res.json({ success: true, data: statuses });
+  const summary = await providerService.checkAllSessions();
+  const failedProviders = Object.keys(summary.failures);
+  if (failedProviders.length > 0) {
+    res.status(503).json({
+      success: false,
+      error: `Failed to check ${failedProviders.length} provider session${failedProviders.length === 1 ? '' : 's'}`,
+      details: summary
+    });
+    return;
+  }
+
+  res.json({ success: true, data: summary.statuses });
 }));
 
 /**
@@ -173,10 +184,17 @@ router.post('/:provider/login-google', async (req: Request, res: Response) => {
 router.get('/:provider/trees', async (req: Request, res: Response) => {
   const { provider } = req.params as { provider: BuiltInProvider };
 
-  const trees = await providerService.discoverTrees(provider)
-    .catch(() => []);
+  const result = await providerService.discoverTrees(provider);
+  if (!result.success) {
+    res.status(503).json({
+      success: false,
+      error: result.error.message,
+      details: result.error
+    });
+    return;
+  }
 
-  res.json({ success: true, data: trees });
+  res.json({ success: true, data: result.data });
 });
 
 /**

server/src/services/provider.service.ts

@@ -6,6 +6,9 @@ import type {
   UserProviderConfig,
   ProviderSessionStatus,
   ProviderTreeInfo,
+  ProviderOperation,
+  ProviderOperationResult,
+  ProviderSessionCheckSummary,
   EnsureAuthResult
 } from '@fsf/shared';
 import { browserService, isFamilySearchAuthUrl } from './browser.service.js';
@@ -16,6 +19,33 @@ import { DATA_DIR } from '../utils/paths.js';
 
 const CONFIG_FILE = path.join(DATA_DIR, 'provider-config.json');
 
+const getErrorMessage = (error: unknown): string =>
+  error instanceof Error ? error.message : String(error);
+
+const runProviderOperation = async <T>(
+  provider: BuiltInProvider,
+  operation: ProviderOperation,
+  action: () => Promise<T>
+): Promise<ProviderOperationResult<T>> => action()
+  .then(data => ({ success: true as const, data }))
+  .catch(error => {
+    const message = getErrorMessage(error);
+    logger.error(
+      'provider-operation',
+      `provider=${provider} operation=${operation} error=${message}`
+    );
+
+    return {
+      success: false as const,
+      error: {
+        code: 'PROVIDER_OPERATION_FAILED' as const,
+        provider,
+        operation,
+        message
+      }
+    };
+  });
+
 /**
  * Create default configuration for all providers
  */
@@ -186,36 +216,36 @@ export const providerService = {
   /**
    * Check browser login status for a provider
    */
-  async checkSession(provider: BuiltInProvider): Promise<ProviderSessionStatus> {
-    const config = this.getConfig(provider);
-    const scraper = getScraper(provider);
-
-    const status: ProviderSessionStatus = {
-      provider,
-      enabled: config.enabled,
-      loggedIn: false,
-      lastChecked: new Date().toISOString()
-    };
+  async checkSession(provider: BuiltInProvider): Promise<ProviderOperationResult<ProviderSessionStatus>> {
+    return runProviderOperation(provider, 'check-session', async () => {
+      const config = this.getConfig(provider);
+      const scraper = getScraper(provider);
 
-    // Ensure browser is connected
-    if (!browserService.isConnected()) {
-      await browserService.connect().catch(() => null);
-    }
+      const status: ProviderSessionStatus = {
+        provider,
+        enabled: config.enabled,
+        loggedIn: false,
+        lastChecked: new Date().toISOString()
+      };
 
-    if (!browserService.isConnected()) {
-      return status;
-    }
+      if (!browserService.isConnected()) {
+        await browserService.connect();
+      }
 
-    const page = await browserService.getWorkerPage();
+      if (!browserService.isConnected()) {
+        throw new Error('Browser connection was not established');
+      }
 
-    status.loggedIn = await scraper.checkLoginStatus(page).catch(() => false);
+      const page = await browserService.getWorkerPage();
+      status.loggedIn = await scraper.checkLoginStatus(page);
 
-    if (status.loggedIn) {
-      const userInfo = await scraper.getLoggedInUser(page).catch(() => null);
-      status.userName = userInfo?.name;
-    }
+      if (status.loggedIn) {
+        const userInfo = await scraper.getLoggedInUser(page).catch(() => null);
+        status.userName = userInfo?.name;
+      }
 
-    return status;
+      return status;
+    });
   },
 
   /**
@@ -327,15 +357,21 @@ export const providerService = {
   /**
    * Check session status for all enabled providers
    */
-  async checkAllSessions(): Promise<Record<BuiltInProvider, ProviderSessionStatus>> {
-    const registry = loadRegistry();
-    const results: Record<BuiltInProvider, ProviderSessionStatus> = {} as Record<BuiltInProvider, ProviderSessionStatus>;
+  async checkAllSessions(): Promise<ProviderSessionCheckSummary> {
+    const registry = this.getAllConfigs();
+    const statuses: ProviderSessionCheckSummary['statuses'] = {};
+    const failures: ProviderSessionCheckSummary['failures'] = {};
 
     for (const provider of listProviders()) {
       if (registry.providers[provider].enabled) {
-        results[provider] = await this.checkSession(provider);
+        const result = await this.checkSession(provider);
+        if (result.success) {
+          statuses[provider] = result.data;
+        } else {
+          failures[provider] = result.error;
+        }
       } else {
-        results[provider] = {
+        statuses[provider] = {
           provider,
           enabled: false,
           loggedIn: false,
@@ -344,23 +380,27 @@ export const providerService = {
       }
     }
 
-    return results;
+    return { statuses, failures };
   },
 
   /**
    * Discover available trees for a provider
    */
-  async discoverTrees(provider: BuiltInProvider): Promise<ProviderTreeInfo[]> {
-    const scraper = getScraper(provider);
+  async discoverTrees(provider: BuiltInProvider): Promise<ProviderOperationResult<ProviderTreeInfo[]>> {
+    return runProviderOperation(provider, 'discover-trees', async () => {
+      const scraper = getScraper(provider);
 
-    if (!browserService.isConnected()) {
-      await browserService.connect();
-    }
+      if (!browserService.isConnected()) {
+        await browserService.connect();
+      }
 
-    const page = await browserService.getWorkerPage();
-    const trees = await scraper.listTrees(page).catch(() => []);
+      if (!browserService.isConnected()) {
+        throw new Error('Browser connection was not established');
+      }
 
-    return trees;
+      const page = await browserService.getWorkerPage();
+      return scraper.listTrees(page);
+    });
   },
 
   /**

shared/src/index.ts

@@ -189,6 +189,24 @@ export interface ProviderTreeInfo {
   rootPersonId?: string;
 }
 
+export type ProviderOperation = 'check-session' | 'discover-trees';
+
+export interface ProviderOperationFailure {
+  code: 'PROVIDER_OPERATION_FAILED';
+  provider: BuiltInProvider;
+  operation: ProviderOperation;
+  message: string;
+}
+
+export type ProviderOperationResult<T> =
+  | { success: true; data: T }
+  | { success: false; error: ProviderOperationFailure };
+
+export interface ProviderSessionCheckSummary {
+  statuses: Partial<Record<BuiltInProvider, ProviderSessionStatus>>;
+  failures: Partial<Record<BuiltInProvider, ProviderOperationFailure>>;
+}
+
 // Auto-login method type
 export type AutoLoginMethod = 'credentials' | 'google';