fix: handle exceptions when fetching specific html url (#1552)

Signed-off-by: Adam Setch <[email protected]>
gitify-app · Sep 27, 2024 · 037af4e · 037af4e
1 parent 1f3f4af
commit 037af4e
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 44 deletions.
diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts
@@ -5,6 +5,7 @@ import {
   mockPersonalAccessTokenAccount,
 } from '../__mocks__/state-mocks';
 
+import log from 'electron-log';
 import { defaultSettings } from '../context/App';
 import type { Hostname, Link, SettingsState } from '../types';
 import type { SubjectType } from '../typesGitHub';
@@ -505,23 +506,57 @@ describe('utils/helpers.ts', () => {
       });
     });
 
-    it('defaults to repository url', async () => {
-      const subject = {
-        title: 'generate github web url unit tests',
-        url: null,
-        latest_comment_url: null,
-        type: 'Issue' as SubjectType,
-      };
+    describe('defaults to repository url', () => {
+      it('defaults when no urls present in notification', async () => {
+        const subject = {
+          title: 'generate github web url unit tests',
+          url: null,
+          latest_comment_url: null,
+          type: 'Issue' as SubjectType,
+        };
 
-      const result = await generateGitHubWebUrl({
-        ...mockSingleNotification,
-        subject: subject,
+        const result = await generateGitHubWebUrl({
+          ...mockSingleNotification,
+          subject: subject,
+        });
+
+        expect(apiRequestAuthMock).toHaveBeenCalledTimes(0);
+        expect(result).toBe(
+          `${mockSingleNotification.repository.html_url}?${mockNotificationReferrer}`,
+        );
       });
 
-      expect(apiRequestAuthMock).toHaveBeenCalledTimes(0);
-      expect(result).toBe(
-        `${mockSingleNotification.repository.html_url}?${mockNotificationReferrer}`,
-      );
+      it('defaults when exception handled during specialized html enrichment process', async () => {
+        const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();
+
+        const subject = {
+          title: 'generate github web url unit tests',
+          url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1' as Link,
+          latest_comment_url:
+            'https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302888448' as Link,
+          type: 'Issue' as SubjectType,
+        };
+
+        const mockError = new Error('Test error');
+
+        apiRequestAuthMock.mockRejectedValue(mockError);
+
+        const result = await generateGitHubWebUrl({
+          ...mockSingleNotification,
+          subject: subject,
+        });
+
+        expect(apiRequestAuthMock).toHaveBeenCalledTimes(1);
+        expect(apiRequestAuthMock).toHaveBeenCalledWith(
+          subject.latest_comment_url,
+          'GET',
+          mockPersonalAccessTokenAccount.token,
+        );
+        expect(result).toBe(
+          `https://github.com/gitify-app/notifications-test?${mockNotificationReferrer}`,
+        );
+        expect(logErrorSpy).toHaveBeenCalledTimes(2);
+      });
     });
   });
 

diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts
@@ -1,4 +1,5 @@
 import { formatDistanceToNowStrict, parseISO } from 'date-fns';
+import log from 'electron-log';
 import { defaultSettings } from '../context/App';
 import type { Account, Hostname, Link, SettingsState } from '../types';
 import type { Notification } from '../typesGitHub';
@@ -117,37 +118,44 @@ export async function generateGitHubWebUrl(
 ): Promise<Link> {
   const url = new URL(notification.repository.html_url);
 
-  if (notification.subject.latest_comment_url) {
-    url.href = await getHtmlUrl(
-      notification.subject.latest_comment_url,
-      notification.account.token,
-    );
-  } else if (notification.subject.url) {
-    url.href = await getHtmlUrl(
-      notification.subject.url,
-      notification.account.token,
-    );
-  } else {
-    // Perform any specific notification type handling (only required for a few special notification scenarios)
-    switch (notification.subject.type) {
-      case 'CheckSuite':
-        url.href = getCheckSuiteUrl(notification);
-        break;
-      case 'Discussion':
-        url.href = await getDiscussionUrl(notification);
-        break;
-      case 'RepositoryInvitation':
-        url.pathname += '/invitations';
-        break;
-      case 'RepositoryDependabotAlertsThread':
-        url.pathname += '/security/dependabot';
-        break;
-      case 'WorkflowRun':
-        url.href = getWorkflowRunUrl(notification);
-        break;
-      default:
-        break;
+  try {
+    if (notification.subject.latest_comment_url) {
+      url.href = await getHtmlUrl(
+        notification.subject.latest_comment_url,
+        notification.account.token,
+      );
+    } else if (notification.subject.url) {
+      url.href = await getHtmlUrl(
+        notification.subject.url,
+        notification.account.token,
+      );
+    } else {
+      // Perform any specific notification type handling (only required for a few special notification scenarios)
+      switch (notification.subject.type) {
+        case 'CheckSuite':
+          url.href = getCheckSuiteUrl(notification);
+          break;
+        case 'Discussion':
+          url.href = await getDiscussionUrl(notification);
+          break;
+        case 'RepositoryInvitation':
+          url.pathname += '/invitations';
+          break;
+        case 'RepositoryDependabotAlertsThread':
+          url.pathname += '/security/dependabot';
+          break;
+        case 'WorkflowRun':
+          url.href = getWorkflowRunUrl(notification);
+          break;
+        default:
+          break;
+      }
     }
+  } catch (err) {
+    log.error(
+      'Error occurred while attempting to get a specific notification URL.  Will fall back to defaults',
+      err,
+    );
   }
 
   url.searchParams.set(