refactor: extract error logger helper

setchy · setchy · commit 1966a8399f49 · 2024-12-28T08:49:48.000+01:00
Signed-off-by: Adam Setch &lt;adam.setch@outlook.com&gt;
diff --git a/src/renderer/hooks/useNotifications.test.ts b/src/renderer/hooks/useNotifications.test.ts
@@ -2,7 +2,6 @@ import { act, renderHook, waitFor } from '@testing-library/react';
 import axios, { AxiosError } from 'axios';
 import nock from 'nock';
 
-import log from 'electron-log';
 import {
   mockAuth,
   mockGitHubCloudAccount,
@@ -14,10 +13,11 @@ import {
   mockSingleNotification,
 } from '../utils/api/__mocks__/response-mocks';
 import { Errors } from '../utils/errors';
+import * as logger from '../utils/logger';
 import { useNotifications } from './useNotifications';
 
 describe('renderer/hooks/useNotifications.ts', () => {
-  const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();
+  const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation();
 
   beforeEach(() => {
     // axios will default to using the XHR adapter which can't be intercepted
diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts
@@ -1,4 +1,3 @@
-import log from 'electron-log';
 import { useCallback, useState } from 'react';
 import type {
   Account,
@@ -14,6 +13,7 @@ import {
   markNotificationThreadAsRead,
 } from '../utils/api/client';
 import { isMarkAsDoneFeatureSupported } from '../utils/features';
+import { logError } from '../utils/logger';
 import {
   getAllNotifications,
   setTrayIconColor,
@@ -121,8 +121,9 @@ export const useNotifications = (): NotificationsState => {
         setNotifications(updatedNotifications);
         setTrayIconColor(updatedNotifications);
       } catch (err) {
-        log.error(
-          '[markNotificationsAsRead]: Error occurred while marking notifications as read',
+        logError(
+          'markNotificationsAsRead',
+          'Error occurred while marking notifications as read',
           err,
         );
       }
@@ -158,8 +159,9 @@ export const useNotifications = (): NotificationsState => {
         setNotifications(updatedNotifications);
         setTrayIconColor(updatedNotifications);
       } catch (err) {
-        log.error(
-          '[markNotificationsAsDone]: error occurred while marking notifications as done',
+        logError(
+          'markNotificationsAsDone',
+          'Error occurred while marking notifications as done',
           err,
         );
       }
@@ -186,9 +188,11 @@ export const useNotifications = (): NotificationsState => {
           await markNotificationsAsRead(state, [notification]);
         }
       } catch (err) {
-        log.error(
-          '[unsubscribeNotification]: error occurred while unsubscribing from notification thread',
+        logError(
+          'unsubscribeNotification',
+          'Error occurred while unsubscribing from notification thread',
           err,
+          notification,
         );
       }
 
diff --git a/src/renderer/routes/Accounts.tsx b/src/renderer/routes/Accounts.tsx
@@ -11,7 +11,6 @@ import {
   SyncIcon,
 } from '@primer/octicons-react';
 
-import log from 'electron-log';
 import { type FC, useCallback, useContext } from 'react';
 import { useNavigate } from 'react-router-dom';
 import { Header } from '../components/Header';
@@ -30,6 +29,7 @@ import {
   openDeveloperSettings,
   openHost,
 } from '../utils/links';
+import { logError } from '../utils/logger';
 import { saveState } from '../utils/storage';
 
 export const AccountsRoute: FC = () => {
@@ -57,7 +57,7 @@ export const AccountsRoute: FC = () => {
     try {
       await loginWithGitHubApp();
     } catch (err) {
-      log.error('Auth: failed to login with GitHub', err);
+      logError('loginWithGitHub', 'failed to login with GitHub', err);
     }
   }, []);
 
diff --git a/src/renderer/routes/Login.tsx b/src/renderer/routes/Login.tsx
@@ -1,12 +1,12 @@
 import { KeyIcon, MarkGithubIcon, PersonIcon } from '@primer/octicons-react';
-import log from 'electron-log';
 import { type FC, useCallback, useContext, useEffect } from 'react';
 import { useNavigate } from 'react-router-dom';
 import { Button } from '../components/buttons/Button';
 import { LogoIcon } from '../components/icons/LogoIcon';
 import { AppContext } from '../context/App';
 import { Size } from '../types';
 import { showWindow } from '../utils/comms';
+import { logError } from '../utils/logger';
 
 export const LoginRoute: FC = () => {
   const navigate = useNavigate();
@@ -23,7 +23,7 @@ export const LoginRoute: FC = () => {
     try {
       await loginWithGitHubApp();
     } catch (err) {
-      log.error('Auth: failed to login with GitHub', err);
+      logError('loginWithGitHubApp', 'failed to login with GitHub', err);
     }
   }, [loginWithGitHubApp]);
 
diff --git a/src/renderer/routes/LoginWithOAuthApp.tsx b/src/renderer/routes/LoginWithOAuthApp.tsx
@@ -1,5 +1,4 @@
 import { BookIcon, PersonIcon, SignInIcon } from '@primer/octicons-react';
-import log from 'electron-log';
 import { type FC, useCallback, useContext } from 'react';
 import { Form, type FormRenderProps } from 'react-final-form';
 import { useNavigate } from 'react-router-dom';
@@ -22,6 +21,7 @@ import {
   isValidToken,
 } from '../utils/auth/utils';
 import { Constants } from '../utils/constants';
+import { logError } from '../utils/logger';
 
 interface IValues {
   hostname?: Hostname;
@@ -131,7 +131,7 @@ export const LoginWithOAuthAppRoute: FC = () => {
         await loginWithOAuthApp(data as LoginOAuthAppOptions);
         navigate(-1);
       } catch (err) {
-        log.error('Auth: Failed to login with oauth app', err);
+        logError('loginWithOAuthApp', 'Failed to login with OAuth App', err);
       }
     },
     [loginWithOAuthApp],
diff --git a/src/renderer/routes/LoginWithPersonalAccessToken.tsx b/src/renderer/routes/LoginWithPersonalAccessToken.tsx
@@ -1,5 +1,4 @@
 import { BookIcon, KeyIcon, SignInIcon } from '@primer/octicons-react';
-import log from 'electron-log';
 import { type FC, useCallback, useContext, useState } from 'react';
 import { Form, type FormRenderProps } from 'react-final-form';
 import { useNavigate } from 'react-router-dom';
@@ -15,6 +14,7 @@ import {
   isValidToken,
 } from '../utils/auth/utils';
 import { Constants } from '../utils/constants';
+import { logError } from '../utils/logger';
 
 interface IValues {
   token?: Token;
@@ -129,7 +129,11 @@ export const LoginWithPersonalAccessTokenRoute: FC = () => {
         );
         navigate(-1);
       } catch (err) {
-        log.error('Auth: failed to login with personal access token', err);
+        logError(
+          'loginWithPersonalAccessToken',
+          'Failed to login with PAT',
+          err,
+        );
         setIsValidToken(false);
       }
     },
diff --git a/src/renderer/utils/api/client.test.ts b/src/renderer/utils/api/client.test.ts
@@ -1,11 +1,11 @@
 import axios, { type AxiosPromise, type AxiosResponse } from 'axios';
-import log from 'electron-log';
 import {
   mockGitHubCloudAccount,
   mockGitHubEnterpriseServerAccount,
   mockToken,
 } from '../../__mocks__/state-mocks';
 import type { Hostname, Link, SettingsState, Token } from '../../types';
+import * as logger from '../logger';
 import {
   getAuthenticatedUser,
   getHtmlUrl,
@@ -268,7 +268,7 @@ describe('renderer/utils/api/client.ts', () => {
     });
 
     it('should handle error', async () => {
-      const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();
+      const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation();
 
       const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth');
 
diff --git a/src/renderer/utils/api/client.ts b/src/renderer/utils/api/client.ts
@@ -1,5 +1,4 @@
 import type { AxiosPromise } from 'axios';
-import log from 'electron-log';
 import { print } from 'graphql/language/printer';
 import type {
   Account,
@@ -23,6 +22,7 @@ import type {
   UserDetails,
 } from '../../typesGitHub';
 import { isAnsweredDiscussionFeatureSupported } from '../features';
+import { logError } from '../logger';
 import { QUERY_SEARCH_DISCUSSIONS } from './graphql/discussions';
 import { formatAsGitHubSearchSyntax } from './graphql/utils';
 import { apiRequestAuth } from './request';
@@ -217,9 +217,9 @@ export async function getHtmlUrl(url: Link, token: Token): Promise<string> {
     const response = (await apiRequestAuth(url, 'GET', token)).data;
     return response.html_url;
   } catch (err) {
-    log.error(
-      '[getHtmlUrl]: error occurred while fetching html url for',
-      url,
+    logError(
+      'getHtmlUrl',
+      `error occurred while fetching html url for ${url}`,
       err,
     );
   }
@@ -271,10 +271,11 @@ export async function getLatestDiscussion(
       )[0] ?? null
     );
   } catch (err) {
-    log.error(
-      '[getLatestDiscussion]: failed to fetch latest discussion for notification',
-      `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`,
+    logError(
+      'getLatestDiscussion',
+      'failed to fetch latest discussion for notification',
       err,
+      notification,
     );
   }
 }
diff --git a/src/renderer/utils/api/request.ts b/src/renderer/utils/api/request.ts
@@ -3,8 +3,8 @@ import axios, {
   type AxiosPromise,
   type Method,
 } from 'axios';
-import log from 'electron-log';
 import type { Link, Token } from '../../types';
+import { logError } from '../logger';
 import { getNextURLFromLinkHeader } from './utils';
 
 /**
@@ -73,7 +73,8 @@ export async function apiRequestAuth(
       nextUrl = getNextURLFromLinkHeader(response);
     }
   } catch (err) {
-    log.error('[apiRequestAuth]: API request failed:', err);
+    logError('apiRequestAuth', 'API request failed:', err);
+
     throw err;
   }
 
diff --git a/src/renderer/utils/auth/utils.ts b/src/renderer/utils/auth/utils.ts
@@ -1,6 +1,5 @@
 import { BrowserWindow } from '@electron/remote';
 import { format } from 'date-fns';
-import log from 'electron-log';
 import semver from 'semver';
 import type {
   Account,
@@ -17,6 +16,7 @@ import { getAuthenticatedUser } from '../api/client';
 import { apiRequest } from '../api/request';
 import { Constants } from '../constants';
 import { getPlatformFromHostname } from '../helpers';
+import { logError } from '../logger';
 import type { AuthMethod, AuthResponse, AuthTokenResponse } from './types';
 
 // TODO - Refactor our OAuth2 flow to use system browser and local app gitify://callback - see #485 #561 #654
@@ -178,9 +178,9 @@ export async function refreshAccount(account: Account): Promise<Account> {
       accountScopes.includes(scope),
     );
   } catch (err) {
-    log.error(
-      '[refreshAccount]: failed to refresh account for user',
-      account.user.login,
+    logError(
+      'refreshAccount',
+      `failed to refresh account for user ${account.user.login}`,
       err,
     );
   }
diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts
@@ -6,7 +6,6 @@ import {
   ChevronLeftIcon,
   ChevronRightIcon,
 } from '@primer/octicons-react';
-import log from 'electron-log';
 import { defaultSettings } from '../context/App';
 import type { Hostname, Link, SettingsState } from '../types';
 import type { SubjectType } from '../typesGitHub';
@@ -15,6 +14,7 @@ import {
   mockSingleNotification,
 } from './api/__mocks__/response-mocks';
 import * as apiRequests from './api/request';
+import * as logger from './logger';
 
 import {
   formatForDisplay,
@@ -487,7 +487,7 @@ describe('renderer/utils/helpers.ts', () => {
       });
 
       it('defaults when exception handled during specialized html enrichment process', async () => {
-        const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();
+        const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation();
 
         const subject = {
           title: 'generate github web url unit tests',
diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts
@@ -11,6 +11,7 @@ import type { Notification } from '../typesGitHub';
 import { getHtmlUrl, getLatestDiscussion } from './api/client';
 import type { PlatformType } from './auth/types';
 import { Constants } from './constants';
+import { logError } from './logger';
 import {
   getCheckSuiteAttributes,
   getLatestDiscussionComment,
@@ -151,11 +152,13 @@ export async function generateGitHubWebUrl(
       }
     }
   } catch (err) {
-    log.error(
-      '[generateGitHubWebUrl]: failed to resolve specific notification html url for',
-      `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`,
+    logError(
+      'generateGitHubWebUrl',
+      'Failed to resolve specific notification html url for',
       err,
+      notification,
     );
+
     log.warn(
       'Will fall back to opening repository root url for',
       notification.repository.full_name,
diff --git a/src/renderer/utils/logger.test.ts b/src/renderer/utils/logger.test.ts
@@ -0,0 +1,30 @@
+import log from 'electron-log';
+import { mockSingleNotification } from './api/__mocks__/response-mocks';
+import { logError } from './logger';
+
+describe('renderer/utils/logger.ts', () => {
+  const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();
+  const mockError = new Error('baz');
+
+  beforeEach(() => {
+    logErrorSpy.mockReset();
+  });
+
+  it('log error without notification', () => {
+    logError('foo', 'bar', mockError);
+
+    expect(logErrorSpy).toHaveBeenCalledTimes(1);
+    expect(logErrorSpy).toHaveBeenCalledWith('[foo]: bar', mockError);
+  });
+
+  it('log error with notification', () => {
+    logError('foo', 'bar', mockError, mockSingleNotification);
+
+    expect(logErrorSpy).toHaveBeenCalledTimes(1);
+    expect(logErrorSpy).toHaveBeenCalledWith(
+      '[foo]: bar',
+      '[Issue]: I am a robot and this is a test! for repository gitify-app/notifications-test',
+      mockError,
+    );
+  });
+});
diff --git a/src/renderer/utils/logger.ts b/src/renderer/utils/logger.ts
@@ -0,0 +1,19 @@
+import log from 'electron-log';
+import type { Notification } from '../typesGitHub';
+
+export function logError(
+  type: string,
+  message: string,
+  err: Error,
+  notification?: Notification,
+) {
+  if (notification) {
+    log.error(
+      `[${type}]: ${message}`,
+      `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`,
+      err,
+    );
+  } else {
+    log.error(`[${type}]: ${message}`, err);
+  }
+}
diff --git a/src/renderer/utils/notifications.ts b/src/renderer/utils/notifications.ts
diff --git a/src/renderer/utils/subject.test.ts b/src/renderer/utils/subject.test.ts
diff --git a/src/renderer/utils/subject.ts b/src/renderer/utils/subject.ts
