From ffef6313d431592d39cc718175b527fc5ae16e29 Mon Sep 17 00:00:00 2001
From: Michel Roegl-Brunner <michel.roegl-brunner@htl-braunau.at>
Date: Fri, 24 Oct 2025 22:23:59 +0200
Subject: [PATCH] Add error reporting for GitHub rate limit errors

- Add specific error handling for GitHub API rate limit (403) errors
- Create RateLimitError with proper error name for identification
- Store last sync error and error time in settings for UI display
- Add error status display in autosync settings modal
- Show user-friendly error messages with GitHub token suggestion
- Clear error status on successful sync
- Update both GitHubJsonService and GitHubService with rate limit error handling

This provides better user feedback when GitHub API rate limits are exceeded
and guides users to set up a GitHub token for higher rate limits.
---
 src/app/_components/GeneralSettingsModal.tsx | 44 +++++++--------
 src/app/api/settings/auto-sync/route.ts      | 15 ++++--
 src/server/services/autoSyncService.js       | 57 ++++++++++++++++----
 src/server/services/github.ts                |  7 ++-
 src/server/services/githubJsonService.ts     | 11 ++--
 5 files changed, 92 insertions(+), 42 deletions(-)

diff --git a/src/app/_components/GeneralSettingsModal.tsx b/src/app/_components/GeneralSettingsModal.tsx
index 7cf3b9f..9a5debb 100644
--- a/src/app/_components/GeneralSettingsModal.tsx
+++ b/src/app/_components/GeneralSettingsModal.tsx
@@ -46,6 +46,8 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
   const [appriseUrls, setAppriseUrls] = useState<string[]>([]);
   const [appriseUrlsText, setAppriseUrlsText] = useState('');
   const [lastAutoSync, setLastAutoSync] = useState('');
+  const [lastAutoSyncError, setLastAutoSyncError] = useState<string | null>(null);
+  const [lastAutoSyncErrorTime, setLastAutoSyncErrorTime] = useState<string | null>(null);
   const [cronValidationError, setCronValidationError] = useState('');
 
   // Load existing settings when modal opens
@@ -311,6 +313,8 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
           setAppriseUrls(settings.appriseUrls ?? []);
           setAppriseUrlsText((settings.appriseUrls ?? []).join('\n'));
           setLastAutoSync(settings.lastAutoSync ?? '');
+          setLastAutoSyncError(settings.lastAutoSyncError ?? null);
+          setLastAutoSyncErrorTime(settings.lastAutoSyncErrorTime ?? null);
         }
       }
     } catch (error) {
@@ -322,17 +326,6 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
     setIsSaving(true);
     setMessage(null);
     
-    console.log('Saving auto-sync settings:', {
-      autoSyncEnabled,
-      syncIntervalType,
-      syncIntervalPredefined,
-      syncIntervalCron,
-      autoDownloadNew,
-      autoUpdateExisting,
-      notificationEnabled,
-      appriseUrls
-    });
-    
     try {
       const response = await fetch('/api/settings/auto-sync', {
         method: 'POST',
@@ -348,17 +341,12 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
           appriseUrls: appriseUrls
         })
       });
-
-      console.log('API response status:', response.status);
       
       if (response.ok) {
-        const result = await response.json();
-        console.log('API response data:', result);
         setMessage({ type: 'success', text: 'Auto-sync settings saved successfully!' });
         setTimeout(() => setMessage(null), 3000);
       } else {
         const errorData = await response.json();
-        console.error('API error:', errorData);
         setMessage({ type: 'error', text: errorData.error ?? 'Failed to save auto-sync settings' });
       }
     } catch (error) {
@@ -817,7 +805,6 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
                     <Toggle
                       checked={autoSyncEnabled}
                       onCheckedChange={async (checked) => {
-                        console.log('Toggle changed to:', checked);
                         setAutoSyncEnabled(checked);
                         
                         // Auto-save when toggle changes
@@ -843,14 +830,10 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
                           });
                           
                           if (response.ok) {
-                            console.log('Auto-sync toggle saved successfully');
                             // Update local state to reflect the effective sync interval type
                             if (effectiveSyncIntervalType !== syncIntervalType) {
                               setSyncIntervalType(effectiveSyncIntervalType);
                             }
-                          } else {
-                            const errorData = await response.json();
-                            console.error('Failed to save auto-sync toggle:', errorData);
                           }
                         } catch (error) {
                           console.error('Error saving auto-sync toggle:', error);
@@ -1047,6 +1030,25 @@ export function GeneralSettingsModal({ isOpen, onClose }: GeneralSettingsModalPr
                         </div>
                       )}
 
+                      {lastAutoSyncError && (
+                        <div className="p-3 bg-error/10 text-error-foreground border border-error/20 rounded-md">
+                          <div className="flex items-start gap-2">
+                            <svg className="w-4 h-4 mt-0.5 flex-shrink-0" fill="currentColor" viewBox="0 0 20 20">
+                              <path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zM8.707 7.293a1 1 0 00-1.414 1.414L8.586 10l-1.293 1.293a1 1 0 101.414 1.414L10 11.414l1.293 1.293a1 1 0 001.414-1.414L11.414 10l1.293-1.293a1 1 0 00-1.414-1.414L10 8.586 8.707 7.293z" clipRule="evenodd" />
+                            </svg>
+                            <div>
+                              <p className="text-sm font-medium">Last sync error:</p>
+                              <p className="text-sm mt-1">{lastAutoSyncError}</p>
+                              {lastAutoSyncErrorTime && (
+                                <p className="text-xs mt-1 opacity-75">
+                                  {new Date(lastAutoSyncErrorTime).toLocaleString()}
+                                </p>
+                              )}
+                            </div>
+                          </div>
+                        </div>
+                      )}
+
                       <div className="flex gap-2">
                         <Button
                           onClick={triggerManualSync}
diff --git a/src/app/api/settings/auto-sync/route.ts b/src/app/api/settings/auto-sync/route.ts
index f912a27..7d22776 100644
--- a/src/app/api/settings/auto-sync/route.ts
+++ b/src/app/api/settings/auto-sync/route.ts
@@ -136,7 +136,9 @@ export async function POST(request: NextRequest) {
       'AUTO_UPDATE_EXISTING': settings.autoUpdateExisting ? 'true' : 'false',
       'NOTIFICATION_ENABLED': settings.notificationEnabled ? 'true' : 'false',
       'APPRISE_URLS': Array.isArray(settings.appriseUrls) ? JSON.stringify(settings.appriseUrls) : (settings.appriseUrls || '[]'),
-      'LAST_AUTO_SYNC': settings.lastAutoSync || ''
+      'LAST_AUTO_SYNC': settings.lastAutoSync || '',
+      'LAST_AUTO_SYNC_ERROR': settings.lastAutoSyncError || '',
+      'LAST_AUTO_SYNC_ERROR_TIME': settings.lastAutoSyncErrorTime || ''
     };
 
     // Update or add each setting
@@ -177,6 +179,9 @@ export async function POST(request: NextRequest) {
         autoSyncService.stopAutoSync();
         // Ensure the service is completely stopped and won't restart
         autoSyncService.isRunning = false;
+        // Also stop the global service instance if it exists
+        const { stopAutoSync: stopGlobalAutoSync } = await import('../../../../server/lib/autoSyncInit.js');
+        stopGlobalAutoSync();
       }
     } catch (error) {
       console.error('Error rescheduling auto-sync service:', error);
@@ -212,7 +217,9 @@ export async function GET() {
           autoUpdateExisting: false,
           notificationEnabled: false,
           appriseUrls: [],
-          lastAutoSync: ''
+          lastAutoSync: '',
+          lastAutoSyncError: null,
+          lastAutoSyncErrorTime: null
         }
       });
     }
@@ -236,7 +243,9 @@ export async function GET() {
           return [];
         }
       })(),
-      lastAutoSync: getEnvValue(envContent, 'LAST_AUTO_SYNC') || ''
+      lastAutoSync: getEnvValue(envContent, 'LAST_AUTO_SYNC') || '',
+      lastAutoSyncError: getEnvValue(envContent, 'LAST_AUTO_SYNC_ERROR') || null,
+      lastAutoSyncErrorTime: getEnvValue(envContent, 'LAST_AUTO_SYNC_ERROR_TIME') || null
     };
     
     return NextResponse.json({ settings });
diff --git a/src/server/services/autoSyncService.js b/src/server/services/autoSyncService.js
index cade526..fec2ec4 100644
--- a/src/server/services/autoSyncService.js
+++ b/src/server/services/autoSyncService.js
@@ -48,7 +48,9 @@ export class AutoSyncService {
         autoUpdateExisting: false,
         notificationEnabled: false,
         appriseUrls: [],
-        lastAutoSync: ''
+        lastAutoSync: '',
+        lastAutoSyncError: null,
+        lastAutoSyncErrorTime: null
       };
       const lines = envContent.split('\n');
       
@@ -93,6 +95,12 @@ export class AutoSyncService {
             case 'LAST_AUTO_SYNC':
               settings.lastAutoSync = value;
               break;
+            case 'LAST_AUTO_SYNC_ERROR':
+              settings.lastAutoSyncError = value;
+              break;
+            case 'LAST_AUTO_SYNC_ERROR_TIME':
+              settings.lastAutoSyncErrorTime = value;
+              break;
           }
         }
       }
@@ -109,7 +117,9 @@ export class AutoSyncService {
         autoUpdateExisting: false,
         notificationEnabled: false,
         appriseUrls: [],
-        lastAutoSync: ''
+        lastAutoSync: '',
+        lastAutoSyncError: null,
+        lastAutoSyncErrorTime: null
       };
     }
   }
@@ -149,7 +159,9 @@ export class AutoSyncService {
         'AUTO_UPDATE_EXISTING': settings.autoUpdateExisting.toString(),
         'NOTIFICATION_ENABLED': settings.notificationEnabled.toString(),
         'APPRISE_URLS': JSON.stringify(settings.appriseUrls || []),
-        'LAST_AUTO_SYNC': settings.lastAutoSync || ''
+        'LAST_AUTO_SYNC': settings.lastAutoSync || '',
+        'LAST_AUTO_SYNC_ERROR': settings.lastAutoSyncError || '',
+        'LAST_AUTO_SYNC_ERROR_TIME': settings.lastAutoSyncErrorTime || ''
       };
       
       const existingKeys = new Set();
@@ -426,9 +438,13 @@ export class AutoSyncService {
         await this.sendSyncNotification(results);
       }
       
-      // Step 4: Update last sync time
+      // Step 4: Update last sync time and clear any previous errors
       const lastSyncTime = this.safeToISOString(new Date());
-      const updatedSettings = { ...settings, lastAutoSync: lastSyncTime };
+      const updatedSettings = { 
+        ...settings, 
+        lastAutoSync: lastSyncTime,
+        lastAutoSyncError: null // Clear any previous errors on successful sync
+      };
       this.saveSettings(updatedSettings);
       
       const duration = new Date().getTime() - startTime.getTime();
@@ -444,13 +460,22 @@ export class AutoSyncService {
     } catch (error) {
       console.error('Auto-sync execution failed:', error);
       
+      // Check if it's a rate limit error
+      const isRateLimitError = error instanceof Error && error.name === 'RateLimitError';
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      
       // Send error notification if enabled
       const settings = this.loadSettings();
       if (settings.notificationEnabled && settings.appriseUrls?.length > 0) {
         try {
+          const notificationTitle = isRateLimitError ? 'Auto-Sync Rate Limited' : 'Auto-Sync Failed';
+          const notificationMessage = isRateLimitError 
+            ? `GitHub API rate limit exceeded. Please set a GITHUB_TOKEN in your .env file for higher rate limits. Error: ${errorMessage}`
+            : `Auto-sync failed with error: ${errorMessage}`;
+            
           await appriseService.sendNotification(
-            'Auto-Sync Failed',
-            `Auto-sync failed with error: ${error instanceof Error ? error.message : String(error)}`,
+            notificationTitle,
+            notificationMessage,
             settings.appriseUrls
           );
         } catch (notifError) {
@@ -458,10 +483,24 @@ export class AutoSyncService {
         }
       }
       
+      // Store the error in settings for UI display
+      const errorSettings = this.loadSettings();
+      const errorToStore = isRateLimitError 
+        ? `GitHub API rate limit exceeded. Please set a GITHUB_TOKEN in your .env file for higher rate limits.`
+        : errorMessage;
+      
+      const updatedErrorSettings = { 
+        ...errorSettings, 
+        lastAutoSyncError: errorToStore,
+        lastAutoSyncErrorTime: this.safeToISOString(new Date())
+      };
+      this.saveSettings(updatedErrorSettings);
+      
       return {
         success: false,
-        message: error instanceof Error ? error.message : String(error),
-        error: error instanceof Error ? error.message : String(error)
+        message: errorToStore,
+        error: errorMessage,
+        isRateLimitError
       };
     } finally {
       this.isRunning = false;
diff --git a/src/server/services/github.ts b/src/server/services/github.ts
index 2545a6e..71324fd 100644
--- a/src/server/services/github.ts
+++ b/src/server/services/github.ts
@@ -37,16 +37,15 @@ export class GitHubService {
     // Add GitHub token authentication if available
     if (env.GITHUB_TOKEN) {
       headers.Authorization = `token ${env.GITHUB_TOKEN}`;
-      console.log('Using GitHub token for API authentication');
-    } else {
-      console.log('No GitHub token found, using unauthenticated requests (lower rate limits)');
     }
     
     const response = await fetch(`${this.baseUrl}${endpoint}`, { headers });
 
     if (!response.ok) {
       if (response.status === 403) {
-        throw new Error(`GitHub API rate limit exceeded. Consider setting GITHUB_TOKEN for higher limits. Status: ${response.status} ${response.statusText}`);
+        const error = new Error(`GitHub API rate limit exceeded. Consider setting GITHUB_TOKEN for higher limits. Status: ${response.status} ${response.statusText}`);
+        error.name = 'RateLimitError';
+        throw error;
       }
       throw new Error(`GitHub API error: ${response.status} ${response.statusText}`);
     }
diff --git a/src/server/services/githubJsonService.ts b/src/server/services/githubJsonService.ts
index 01a6f82..f9cd99d 100644
--- a/src/server/services/githubJsonService.ts
+++ b/src/server/services/githubJsonService.ts
@@ -50,16 +50,15 @@ export class GitHubJsonService {
     // Add GitHub token authentication if available
     if (env.GITHUB_TOKEN) {
       headers.Authorization = `token ${env.GITHUB_TOKEN}`;
-      console.log('Using GitHub token for API authentication');
-    } else {
-      console.log('No GitHub token found, using unauthenticated requests (lower rate limits)');
     }
     
     const response = await fetch(`${this.baseUrl!}${endpoint}`, { headers });
 
     if (!response.ok) {
       if (response.status === 403) {
-        throw new Error(`GitHub API rate limit exceeded. Consider setting GITHUB_TOKEN for higher limits. Status: ${response.status} ${response.statusText}`);
+        const error = new Error(`GitHub API rate limit exceeded. Consider setting GITHUB_TOKEN for higher limits. Status: ${response.status} ${response.statusText}`);
+        error.name = 'RateLimitError';
+        throw error;
       }
       throw new Error(`GitHub API error: ${response.status} ${response.statusText}`);
     }
@@ -83,7 +82,9 @@ export class GitHubJsonService {
     const response = await fetch(rawUrl, { headers });
     if (!response.ok) {
       if (response.status === 403) {
-        throw new Error(`GitHub rate limit exceeded while downloading ${filePath}. Consider setting GITHUB_TOKEN for higher limits. Status: ${response.status} ${response.statusText}`);
+        const error = new Error(`GitHub rate limit exceeded while downloading ${filePath}. Consider setting GITHUB_TOKEN for higher limits. Status: ${response.status} ${response.statusText}`);
+        error.name = 'RateLimitError';
+        throw error;
       }
       throw new Error(`Failed to download ${filePath}: ${response.status} ${response.statusText}`);
     }