From 1966a8399f49a99e8d41216df88068d03bc7af01 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 28 Dec 2024 08:49:48 +0100 Subject: [PATCH 1/6] refactor: extract error logger helper Signed-off-by: Adam Setch --- src/renderer/hooks/useNotifications.test.ts | 4 +-- src/renderer/hooks/useNotifications.ts | 18 ++++++----- src/renderer/routes/Accounts.tsx | 4 +-- src/renderer/routes/Login.tsx | 4 +-- src/renderer/routes/LoginWithOAuthApp.tsx | 4 +-- .../routes/LoginWithPersonalAccessToken.tsx | 8 +++-- src/renderer/utils/api/client.test.ts | 4 +-- src/renderer/utils/api/client.ts | 15 +++++----- src/renderer/utils/api/request.ts | 5 ++-- src/renderer/utils/auth/utils.ts | 8 ++--- src/renderer/utils/helpers.test.ts | 4 +-- src/renderer/utils/helpers.ts | 9 ++++-- src/renderer/utils/logger.test.ts | 30 +++++++++++++++++++ src/renderer/utils/logger.ts | 19 ++++++++++++ src/renderer/utils/notifications.ts | 15 ++++++---- src/renderer/utils/subject.test.ts | 9 +++--- src/renderer/utils/subject.ts | 9 +++--- 17 files changed, 119 insertions(+), 50 deletions(-) create mode 100644 src/renderer/utils/logger.test.ts create mode 100644 src/renderer/utils/logger.ts diff --git a/src/renderer/hooks/useNotifications.test.ts b/src/renderer/hooks/useNotifications.test.ts index 96b5437d8..166211d90 100644 --- 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 index 3f4d06d58..0b01044c9 100644 --- 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 index 2238401e4..e14aff85c 100644 --- 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 index 2a661eb67..432911679 100644 --- a/src/renderer/routes/Login.tsx +++ b/src/renderer/routes/Login.tsx @@ -1,5 +1,4 @@ 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'; @@ -7,6 +6,7 @@ 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 index de44218d2..51dd11405 100644 --- 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 index cebc284fd..8481d2950 100644 --- 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 index ccb7ad2c5..01e12e2ed 100644 --- 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 index bf39f204f..78fab598c 100644 --- 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 { 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 index 3e0da29e9..8b3295825 100644 --- 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 index 22d114ac4..a1bf1a0a7 100644 --- 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 { 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 index 42547328c..8dae48659 100644 --- 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 index 2017017e5..2ffcbd361 100644 --- 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 new file mode 100644 index 000000000..3bfe89d26 --- /dev/null +++ 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 new file mode 100644 index 000000000..f84ebf9c5 --- /dev/null +++ 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 index 1b0a41b0b..fdfffeb97 100644 --- a/src/renderer/utils/notifications.ts +++ b/src/renderer/utils/notifications.ts @@ -12,6 +12,7 @@ import { getAccountUUID } from './auth/utils'; import { hideWindow, showWindow, updateTrayIcon } from './comms'; import { Constants } from './constants'; import { openNotification } from './links'; +import { logError } from './logger'; import { isWindows } from './platform'; import { getGitifySubjectDetails } from './subject'; @@ -157,10 +158,12 @@ export async function getAllNotifications( error: null, }; } catch (err) { - log.error( - '[getAllNotifications]: error occurred while fetching account notifications', + logError( + 'getAllNotifications', + 'error occurred while fetching account notifications', err, ); + return { account: accountNotifications.account, notifications: [], @@ -188,11 +191,13 @@ export async function enrichNotifications( try { additionalSubjectDetails = await getGitifySubjectDetails(notification); } catch (err) { - log.error( - '[enrichNotifications] failed to enrich notification details for', - `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, + logError( + 'enrichNotifications', + 'failed to enrich notification details for', err, + notification, ); + log.warn('Continuing with base notification details'); } diff --git a/src/renderer/utils/subject.test.ts b/src/renderer/utils/subject.test.ts index a925915ee..c42d7ee2a 100644 --- a/src/renderer/utils/subject.test.ts +++ b/src/renderer/utils/subject.test.ts @@ -31,7 +31,7 @@ const mockDiscussionAuthor: DiscussionAuthor = { avatar_url: 'https://avatars.githubusercontent.com/u/123456789?v=4' as Link, type: 'User', }; -import log from 'electron-log'; +import * as logger from './logger'; describe('renderer/utils/subject.ts', () => { beforeEach(() => { @@ -1152,7 +1152,7 @@ describe('renderer/utils/subject.ts', () => { describe('Error', () => { it('catches error and logs message', async () => { - const logErrorSpy = jest.spyOn(log, 'error').mockImplementation(); + const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation(); const mockError = new Error('Test error'); const mockNotification = partialMockNotification({ @@ -1172,9 +1172,10 @@ describe('renderer/utils/subject.ts', () => { await getGitifySubjectDetails(mockNotification); expect(logErrorSpy).toHaveBeenCalledWith( - '[getGitifySubjectDetails]: failed to fetch details for notification for', - '[Issue]: This issue will throw an error for repository gitify-app/notifications-test', + 'getGitifySubjectDetails', + 'failed to fetch details for notification for', mockError, + mockNotification, ); }); }); diff --git a/src/renderer/utils/subject.ts b/src/renderer/utils/subject.ts index 223ef60ed..5783f7907 100644 --- a/src/renderer/utils/subject.ts +++ b/src/renderer/utils/subject.ts @@ -1,4 +1,3 @@ -import log from 'electron-log'; import type { Link } from '../types'; import type { CheckSuiteAttributes, @@ -25,6 +24,7 @@ import { getPullRequestReviews, getRelease, } from './api/client'; +import { logError } from './logger'; export async function getGitifySubjectDetails( notification: Notification, @@ -49,10 +49,11 @@ export async function getGitifySubjectDetails( return null; } } catch (err) { - log.error( - '[getGitifySubjectDetails]: failed to fetch details for notification for', - `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, + logError( + 'getGitifySubjectDetails', + 'failed to fetch details for notification for', err, + notification, ); } } From 22ffd1a25b0c26f124d1e6cabec5c368b69a54d4 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 29 Dec 2024 07:07:27 +0100 Subject: [PATCH 2/6] refactor: extract all loggers to helper Signed-off-by: Adam Setch --- src/renderer/utils/auth/migration.ts | 13 +++-- src/renderer/utils/helpers.ts | 9 ++-- src/renderer/utils/logger.test.ts | 74 +++++++++++++++++++++++----- src/renderer/utils/logger.ts | 48 +++++++++++++++--- src/renderer/utils/notifications.ts | 8 +-- 5 files changed, 122 insertions(+), 30 deletions(-) diff --git a/src/renderer/utils/auth/migration.ts b/src/renderer/utils/auth/migration.ts index 87b32e678..03947dd6f 100644 --- a/src/renderer/utils/auth/migration.ts +++ b/src/renderer/utils/auth/migration.ts @@ -1,6 +1,6 @@ -import log from 'electron-log'; import type { Account, AuthState } from '../../types'; import { Constants } from '../constants'; +import { logInfo } from '../logger'; import { loadState, saveState } from '../storage'; import { getUserData } from './utils'; @@ -16,7 +16,10 @@ export async function migrateAuthenticatedAccounts() { return; } - log.info('Account Migration: Commencing authenticated accounts migration'); + logInfo( + 'migrateAuthenticatedAccounts', + 'Commencing authenticated accounts migration', + ); const migratedAccounts = await convertAccounts(existing.auth); @@ -24,7 +27,11 @@ export async function migrateAuthenticatedAccounts() { auth: { ...existing.auth, accounts: migratedAccounts }, settings: existing.settings, }); - log.info('Account Migration: Authenticated accounts migration complete'); + + logInfo( + 'migrateAuthenticatedAccounts', + 'Authenticated accounts migration complete', + ); } export function hasAccountsToMigrate(existingAuthState: AuthState): boolean { diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts index 2ffcbd361..7811323ba 100644 --- a/src/renderer/utils/helpers.ts +++ b/src/renderer/utils/helpers.ts @@ -4,14 +4,13 @@ import { ChevronRightIcon, } from '@primer/octicons-react'; import { formatDistanceToNowStrict, parseISO } from 'date-fns'; -import log from 'electron-log'; import { defaultSettings } from '../context/App'; import type { Chevron, Hostname, Link, SettingsState } from '../types'; 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 { logError, logWarn } from './logger'; import { getCheckSuiteAttributes, getLatestDiscussionComment, @@ -159,9 +158,9 @@ export async function generateGitHubWebUrl( notification, ); - log.warn( - 'Will fall back to opening repository root url for', - notification.repository.full_name, + logWarn( + 'generateGitHubWebUrl', + `Falling back to repository root url: ${notification.repository.full_name}`, ); } diff --git a/src/renderer/utils/logger.test.ts b/src/renderer/utils/logger.test.ts index 3bfe89d26..1fc4d3788 100644 --- a/src/renderer/utils/logger.test.ts +++ b/src/renderer/utils/logger.test.ts @@ -1,30 +1,78 @@ import log from 'electron-log'; import { mockSingleNotification } from './api/__mocks__/response-mocks'; -import { logError } from './logger'; +import { logError, logInfo, logWarn } from './logger'; describe('renderer/utils/logger.ts', () => { + const logInfoSpy = jest.spyOn(log, 'info').mockImplementation(); + const logWarnSpy = jest.spyOn(log, 'warn').mockImplementation(); const logErrorSpy = jest.spyOn(log, 'error').mockImplementation(); + const mockError = new Error('baz'); beforeEach(() => { + logInfoSpy.mockReset(); + logWarnSpy.mockReset(); logErrorSpy.mockReset(); }); - it('log error without notification', () => { - logError('foo', 'bar', mockError); + describe('logInfo', () => { + it('log info without notification', () => { + logInfo('foo', 'bar'); + + expect(logInfoSpy).toHaveBeenCalledTimes(1); + expect(logInfoSpy).toHaveBeenCalledWith('[foo]:', 'bar'); + }); + + it('log info with notification', () => { + logInfo('foo', 'bar', mockSingleNotification); + + expect(logInfoSpy).toHaveBeenCalledTimes(1); + expect(logInfoSpy).toHaveBeenCalledWith( + '[foo]:', + 'bar', + '[Issue]: I am a robot and this is a test! for repository gitify-app/notifications-test', + ); + }); + }); + + describe('logWarn', () => { + it('log warn without notification', () => { + logWarn('foo', 'bar'); + + expect(logWarnSpy).toHaveBeenCalledTimes(1); + expect(logWarnSpy).toHaveBeenCalledWith('[foo]:', 'bar'); + }); + + it('log warn with notification', () => { + logWarn('foo', 'bar', mockSingleNotification); - expect(logErrorSpy).toHaveBeenCalledTimes(1); - expect(logErrorSpy).toHaveBeenCalledWith('[foo]: bar', mockError); + expect(logWarnSpy).toHaveBeenCalledTimes(1); + expect(logWarnSpy).toHaveBeenCalledWith( + '[foo]:', + 'bar', + '[Issue]: I am a robot and this is a test! for repository gitify-app/notifications-test', + ); + }); }); - it('log error with notification', () => { - logError('foo', 'bar', mockError, mockSingleNotification); + describe('logError', () => { + 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, - ); + 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 index f84ebf9c5..ecb380bb2 100644 --- a/src/renderer/utils/logger.ts +++ b/src/renderer/utils/logger.ts @@ -1,19 +1,55 @@ import log from 'electron-log'; import type { Notification } from '../typesGitHub'; -export function logError( +function logMessage( + // biome-ignore lint/suspicious/noExplicitAny: + logFunction: (...params: any[]) => void, type: string, message: string, - err: Error, + err?: Error, notification?: Notification, ) { - if (notification) { - log.error( - `[${type}]: ${message}`, + if (notification && err) { + logFunction( + `[${type}]:`, + message, `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, err, ); + } else if (notification) { + logFunction( + `[${type}]:`, + message, + `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, + ); + } else if (err) { + logFunction(`[${type}]:`, message, err); } else { - log.error(`[${type}]: ${message}`, err); + logFunction(`[${type}]:`, message); } } + +export function logInfo( + type: string, + message: string, + notification?: Notification, +) { + logMessage(log.info, type, message, null, notification); +} + +export function logWarn( + type: string, + message: string, + notification?: Notification, +) { + logMessage(log.warn, type, message, null, notification); +} + +export function logError( + type: string, + message: string, + err: Error, + notification?: Notification, +) { + logMessage(log.error, type, message, err, notification); +} diff --git a/src/renderer/utils/notifications.ts b/src/renderer/utils/notifications.ts index fdfffeb97..126d0ae00 100644 --- a/src/renderer/utils/notifications.ts +++ b/src/renderer/utils/notifications.ts @@ -1,5 +1,4 @@ import path from 'node:path'; -import log from 'electron-log'; import type { AccountNotifications, GitifyState, @@ -12,7 +11,7 @@ import { getAccountUUID } from './auth/utils'; import { hideWindow, showWindow, updateTrayIcon } from './comms'; import { Constants } from './constants'; import { openNotification } from './links'; -import { logError } from './logger'; +import { logError, logWarn } from './logger'; import { isWindows } from './platform'; import { getGitifySubjectDetails } from './subject'; @@ -198,7 +197,10 @@ export async function enrichNotifications( notification, ); - log.warn('Continuing with base notification details'); + logWarn( + 'enrichNotifications', + 'Continuing with base notification details', + ); } return { From 496cf1733482671499bde783d6432ad7949f6dd7 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 29 Dec 2024 17:18:43 +0100 Subject: [PATCH 3/6] refactor: extract all loggers to helper Signed-off-by: Adam Setch --- src/renderer/utils/logger.ts | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/renderer/utils/logger.ts b/src/renderer/utils/logger.ts index ecb380bb2..fdfc01a79 100644 --- a/src/renderer/utils/logger.ts +++ b/src/renderer/utils/logger.ts @@ -9,24 +9,19 @@ function logMessage( err?: Error, notification?: Notification, ) { - if (notification && err) { - logFunction( - `[${type}]:`, - message, - `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, - err, - ); - } else if (notification) { - logFunction( - `[${type}]:`, - message, + const args: (string | Error)[] = [`[${type}]:`, message]; + + if (notification) { + args.push( `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, ); - } else if (err) { - logFunction(`[${type}]:`, message, err); - } else { - logFunction(`[${type}]:`, message); } + + if (err) { + args.push(err); + } + + logFunction(...args); } export function logInfo( From d3c1530700c72f96fbe49c69b1f19b2d66d47bf1 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 29 Dec 2024 17:31:57 +0100 Subject: [PATCH 4/6] move to shared folder Signed-off-by: Adam Setch --- src/main/first-run.ts | 5 +++-- src/main/updater.ts | 14 ++++++++------ src/main/utils.ts | 6 ++++-- src/renderer/hooks/useNotifications.test.ts | 2 +- src/renderer/hooks/useNotifications.ts | 3 ++- src/renderer/routes/Accounts.tsx | 4 ++-- src/renderer/routes/Login.tsx | 3 ++- src/renderer/routes/LoginWithOAuthApp.tsx | 3 ++- .../routes/LoginWithPersonalAccessToken.tsx | 3 ++- src/renderer/utils/api/client.test.ts | 2 +- src/renderer/utils/api/client.ts | 3 ++- src/renderer/utils/api/request.ts | 3 ++- src/renderer/utils/auth/migration.ts | 2 +- src/renderer/utils/auth/utils.ts | 3 ++- src/renderer/utils/helpers.test.ts | 2 +- src/renderer/utils/helpers.ts | 3 ++- src/renderer/utils/notifications.ts | 3 ++- src/renderer/utils/subject.test.ts | 2 +- src/renderer/utils/subject.ts | 2 +- src/{renderer/utils => shared}/logger.test.ts | 3 ++- src/{renderer/utils => shared}/logger.ts | 5 +++-- 21 files changed, 46 insertions(+), 30 deletions(-) rename src/{renderer/utils => shared}/logger.test.ts (96%) rename src/{renderer/utils => shared}/logger.ts (92%) diff --git a/src/main/first-run.ts b/src/main/first-run.ts index 0a5678976..c119ea37a 100644 --- a/src/main/first-run.ts +++ b/src/main/first-run.ts @@ -1,7 +1,8 @@ import fs from 'node:fs'; import path from 'node:path'; import { app, dialog } from 'electron'; -import log from 'electron-log'; + +import { logError } from '../shared/logger'; export async function onFirstRunMaybe() { if (isFirstRun()) { @@ -49,7 +50,7 @@ function isFirstRun() { fs.writeFileSync(configPath, ''); } catch (err) { - log.error('First run: Unable to write firstRun file', err); + logError('isFirstRun', 'Unable to write firstRun file', err); } return true; diff --git a/src/main/updater.ts b/src/main/updater.ts index 82b4f5a77..949eae512 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -2,6 +2,8 @@ import log from 'electron-log'; import { autoUpdater } from 'electron-updater'; import type { Menubar } from 'menubar'; import { updateElectronApp } from 'update-electron-app'; + +import { logError, logInfo } from '../shared/logger'; import type MenuBuilder from './menu'; export default class Updater { @@ -18,29 +20,29 @@ export default class Updater { }); autoUpdater.on('checking-for-update', () => { - log.info('Auto Updater: Checking for update'); + logInfo('auto updater', 'Checking for update'); this.menuBuilder.setCheckForUpdatesMenuEnabled(false); }); - autoUpdater.on('error', (error) => { - log.error('Auto Updater: error checking for update', error); + autoUpdater.on('error', (err) => { + logError('auto updater', 'Error checking for update', err); this.menuBuilder.setCheckForUpdatesMenuEnabled(true); }); autoUpdater.on('update-available', () => { - log.info('Auto Updater: New update available'); + logInfo('auto updater', 'New update available'); menuBuilder.setUpdateAvailableMenuEnabled(true); this.menubar.tray.setToolTip('Gitify\nA new update is available'); }); autoUpdater.on('update-downloaded', () => { - log.info('Auto Updater: Update downloaded'); + logInfo('auto updater', 'Update downloaded'); menuBuilder.setUpdateReadyForInstallMenuEnabled(true); this.menubar.tray.setToolTip('Gitify\nA new update is ready to install'); }); autoUpdater.on('update-not-available', () => { - log.info('Auto Updater: update not available'); + logInfo('auto updater', 'Update not available'); this.menuBuilder.setCheckForUpdatesMenuEnabled(true); }); } diff --git a/src/main/utils.ts b/src/main/utils.ts index f86350bf4..a3854d7d2 100644 --- a/src/main/utils.ts +++ b/src/main/utils.ts @@ -5,6 +5,8 @@ import { dialog, shell } from 'electron'; import log from 'electron-log'; import type { Menubar } from 'menubar'; +import { logError, logInfo } from '../shared/logger'; + export function takeScreenshot(mb: Menubar) { const date = new Date(); const dateStr = date.toISOString().replace(/:/g, '-'); @@ -12,7 +14,7 @@ export function takeScreenshot(mb: Menubar) { const capturedPicFilePath = `${os.homedir()}/${dateStr}-gitify-screenshot.png`; mb.window.capturePage().then((img) => { fs.writeFile(capturedPicFilePath, img.toPNG(), () => - log.info(`Screenshot saved ${capturedPicFilePath}`), + logInfo('takeScreenshot', `Screenshot saved ${capturedPicFilePath}`), ); }); } @@ -41,7 +43,7 @@ export function openLogsDirectory() { const logDirectory = path.dirname(log.transports.file?.getFile()?.path); if (!logDirectory) { - log.error('Could not find log directory!'); + logError('openLogsDirectory', 'Could not find log directory!'); return; } diff --git a/src/renderer/hooks/useNotifications.test.ts b/src/renderer/hooks/useNotifications.test.ts index 166211d90..be6a26e48 100644 --- a/src/renderer/hooks/useNotifications.test.ts +++ b/src/renderer/hooks/useNotifications.test.ts @@ -2,6 +2,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import axios, { AxiosError } from 'axios'; import nock from 'nock'; +import * as logger from '../../shared/logger'; import { mockAuth, mockGitHubCloudAccount, @@ -13,7 +14,6 @@ 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', () => { diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index 0b01044c9..f57b35108 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -1,4 +1,6 @@ import { useCallback, useState } from 'react'; + +import { logError } from '../../shared/logger'; import type { Account, AccountNotifications, @@ -13,7 +15,6 @@ import { markNotificationThreadAsRead, } from '../utils/api/client'; import { isMarkAsDoneFeatureSupported } from '../utils/features'; -import { logError } from '../utils/logger'; import { getAllNotifications, setTrayIconColor, diff --git a/src/renderer/routes/Accounts.tsx b/src/renderer/routes/Accounts.tsx index e14aff85c..fbd14122b 100644 --- a/src/renderer/routes/Accounts.tsx +++ b/src/renderer/routes/Accounts.tsx @@ -10,9 +10,10 @@ import { StarIcon, SyncIcon, } from '@primer/octicons-react'; - import { type FC, useCallback, useContext } from 'react'; import { useNavigate } from 'react-router-dom'; + +import { logError } from '../../shared/logger'; import { Header } from '../components/Header'; import { AuthMethodIcon } from '../components/icons/AuthMethodIcon'; import { AvatarIcon } from '../components/icons/AvatarIcon'; @@ -29,7 +30,6 @@ import { openDeveloperSettings, openHost, } from '../utils/links'; -import { logError } from '../utils/logger'; import { saveState } from '../utils/storage'; export const AccountsRoute: FC = () => { diff --git a/src/renderer/routes/Login.tsx b/src/renderer/routes/Login.tsx index 432911679..318f5f58a 100644 --- a/src/renderer/routes/Login.tsx +++ b/src/renderer/routes/Login.tsx @@ -1,12 +1,13 @@ import { KeyIcon, MarkGithubIcon, PersonIcon } from '@primer/octicons-react'; import { type FC, useCallback, useContext, useEffect } from 'react'; import { useNavigate } from 'react-router-dom'; + +import { logError } from '../../shared/logger'; 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(); diff --git a/src/renderer/routes/LoginWithOAuthApp.tsx b/src/renderer/routes/LoginWithOAuthApp.tsx index 51dd11405..51e3dc7e6 100644 --- a/src/renderer/routes/LoginWithOAuthApp.tsx +++ b/src/renderer/routes/LoginWithOAuthApp.tsx @@ -2,6 +2,8 @@ import { BookIcon, PersonIcon, SignInIcon } from '@primer/octicons-react'; import { type FC, useCallback, useContext } from 'react'; import { Form, type FormRenderProps } from 'react-final-form'; import { useNavigate } from 'react-router-dom'; + +import { logError } from '../../shared/logger'; import { Header } from '../components/Header'; import { Button } from '../components/buttons/Button'; import { FieldInput } from '../components/fields/FieldInput'; @@ -21,7 +23,6 @@ import { isValidToken, } from '../utils/auth/utils'; import { Constants } from '../utils/constants'; -import { logError } from '../utils/logger'; interface IValues { hostname?: Hostname; diff --git a/src/renderer/routes/LoginWithPersonalAccessToken.tsx b/src/renderer/routes/LoginWithPersonalAccessToken.tsx index 8481d2950..169df1a37 100644 --- a/src/renderer/routes/LoginWithPersonalAccessToken.tsx +++ b/src/renderer/routes/LoginWithPersonalAccessToken.tsx @@ -2,6 +2,8 @@ import { BookIcon, KeyIcon, SignInIcon } from '@primer/octicons-react'; import { type FC, useCallback, useContext, useState } from 'react'; import { Form, type FormRenderProps } from 'react-final-form'; import { useNavigate } from 'react-router-dom'; + +import { logError } from '../../shared/logger'; import { Header } from '../components/Header'; import { Button } from '../components/buttons/Button'; import { FieldInput } from '../components/fields/FieldInput'; @@ -14,7 +16,6 @@ import { isValidToken, } from '../utils/auth/utils'; import { Constants } from '../utils/constants'; -import { logError } from '../utils/logger'; interface IValues { token?: Token; diff --git a/src/renderer/utils/api/client.test.ts b/src/renderer/utils/api/client.test.ts index 01e12e2ed..f7b9acbd2 100644 --- 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 * as logger from '../../../shared/logger'; import { mockGitHubCloudAccount, mockGitHubEnterpriseServerAccount, mockToken, } from '../../__mocks__/state-mocks'; import type { Hostname, Link, SettingsState, Token } from '../../types'; -import * as logger from '../logger'; import { getAuthenticatedUser, getHtmlUrl, diff --git a/src/renderer/utils/api/client.ts b/src/renderer/utils/api/client.ts index 78fab598c..18b396128 100644 --- a/src/renderer/utils/api/client.ts +++ b/src/renderer/utils/api/client.ts @@ -1,5 +1,7 @@ import type { AxiosPromise } from 'axios'; import { print } from 'graphql/language/printer'; + +import { logError } from '../../../shared/logger'; import type { Account, Hostname, @@ -22,7 +24,6 @@ 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'; diff --git a/src/renderer/utils/api/request.ts b/src/renderer/utils/api/request.ts index 8b3295825..39933e4fb 100644 --- a/src/renderer/utils/api/request.ts +++ b/src/renderer/utils/api/request.ts @@ -3,8 +3,9 @@ import axios, { type AxiosPromise, type Method, } from 'axios'; + +import { logError } from '../../../shared/logger'; import type { Link, Token } from '../../types'; -import { logError } from '../logger'; import { getNextURLFromLinkHeader } from './utils'; /** diff --git a/src/renderer/utils/auth/migration.ts b/src/renderer/utils/auth/migration.ts index 03947dd6f..f662ab7a1 100644 --- a/src/renderer/utils/auth/migration.ts +++ b/src/renderer/utils/auth/migration.ts @@ -1,6 +1,6 @@ +import { logInfo } from '../../../shared/logger'; import type { Account, AuthState } from '../../types'; import { Constants } from '../constants'; -import { logInfo } from '../logger'; import { loadState, saveState } from '../storage'; import { getUserData } from './utils'; diff --git a/src/renderer/utils/auth/utils.ts b/src/renderer/utils/auth/utils.ts index a1bf1a0a7..c121ee0fd 100644 --- a/src/renderer/utils/auth/utils.ts +++ b/src/renderer/utils/auth/utils.ts @@ -1,6 +1,8 @@ import { BrowserWindow } from '@electron/remote'; import { format } from 'date-fns'; import semver from 'semver'; + +import { logError } from '../../../shared/logger'; import type { Account, AuthCode, @@ -16,7 +18,6 @@ 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 diff --git a/src/renderer/utils/helpers.test.ts b/src/renderer/utils/helpers.test.ts index 8dae48659..8bb76ff99 100644 --- a/src/renderer/utils/helpers.test.ts +++ b/src/renderer/utils/helpers.test.ts @@ -6,6 +6,7 @@ import { ChevronLeftIcon, ChevronRightIcon, } from '@primer/octicons-react'; +import * as logger from '../../shared/logger'; import { defaultSettings } from '../context/App'; import type { Hostname, Link, SettingsState } from '../types'; import type { SubjectType } from '../typesGitHub'; @@ -14,7 +15,6 @@ import { mockSingleNotification, } from './api/__mocks__/response-mocks'; import * as apiRequests from './api/request'; -import * as logger from './logger'; import { formatForDisplay, diff --git a/src/renderer/utils/helpers.ts b/src/renderer/utils/helpers.ts index 7811323ba..6954f13e6 100644 --- a/src/renderer/utils/helpers.ts +++ b/src/renderer/utils/helpers.ts @@ -4,13 +4,14 @@ import { ChevronRightIcon, } from '@primer/octicons-react'; import { formatDistanceToNowStrict, parseISO } from 'date-fns'; + +import { logError, logWarn } from '../../shared/logger'; import { defaultSettings } from '../context/App'; import type { Chevron, Hostname, Link, SettingsState } from '../types'; import type { Notification } from '../typesGitHub'; import { getHtmlUrl, getLatestDiscussion } from './api/client'; import type { PlatformType } from './auth/types'; import { Constants } from './constants'; -import { logError, logWarn } from './logger'; import { getCheckSuiteAttributes, getLatestDiscussionComment, diff --git a/src/renderer/utils/notifications.ts b/src/renderer/utils/notifications.ts index 126d0ae00..091f988bc 100644 --- a/src/renderer/utils/notifications.ts +++ b/src/renderer/utils/notifications.ts @@ -1,4 +1,6 @@ import path from 'node:path'; + +import { logError, logWarn } from '../../shared/logger'; import type { AccountNotifications, GitifyState, @@ -11,7 +13,6 @@ import { getAccountUUID } from './auth/utils'; import { hideWindow, showWindow, updateTrayIcon } from './comms'; import { Constants } from './constants'; import { openNotification } from './links'; -import { logError, logWarn } from './logger'; import { isWindows } from './platform'; import { getGitifySubjectDetails } from './subject'; diff --git a/src/renderer/utils/subject.test.ts b/src/renderer/utils/subject.test.ts index c42d7ee2a..d2f42843f 100644 --- a/src/renderer/utils/subject.test.ts +++ b/src/renderer/utils/subject.test.ts @@ -31,7 +31,7 @@ const mockDiscussionAuthor: DiscussionAuthor = { avatar_url: 'https://avatars.githubusercontent.com/u/123456789?v=4' as Link, type: 'User', }; -import * as logger from './logger'; +import * as logger from '../../shared/logger'; describe('renderer/utils/subject.ts', () => { beforeEach(() => { diff --git a/src/renderer/utils/subject.ts b/src/renderer/utils/subject.ts index 5783f7907..cae5cdbfc 100644 --- a/src/renderer/utils/subject.ts +++ b/src/renderer/utils/subject.ts @@ -1,3 +1,4 @@ +import { logError } from '../../shared/logger'; import type { Link } from '../types'; import type { CheckSuiteAttributes, @@ -24,7 +25,6 @@ import { getPullRequestReviews, getRelease, } from './api/client'; -import { logError } from './logger'; export async function getGitifySubjectDetails( notification: Notification, diff --git a/src/renderer/utils/logger.test.ts b/src/shared/logger.test.ts similarity index 96% rename from src/renderer/utils/logger.test.ts rename to src/shared/logger.test.ts index 1fc4d3788..cb0b6f0a0 100644 --- a/src/renderer/utils/logger.test.ts +++ b/src/shared/logger.test.ts @@ -1,5 +1,6 @@ import log from 'electron-log'; -import { mockSingleNotification } from './api/__mocks__/response-mocks'; + +import { mockSingleNotification } from '../renderer/utils/api/__mocks__/response-mocks'; import { logError, logInfo, logWarn } from './logger'; describe('renderer/utils/logger.ts', () => { diff --git a/src/renderer/utils/logger.ts b/src/shared/logger.ts similarity index 92% rename from src/renderer/utils/logger.ts rename to src/shared/logger.ts index fdfc01a79..972e6af53 100644 --- a/src/renderer/utils/logger.ts +++ b/src/shared/logger.ts @@ -1,5 +1,6 @@ import log from 'electron-log'; -import type { Notification } from '../typesGitHub'; + +import type { Notification } from '../renderer/typesGitHub'; function logMessage( // biome-ignore lint/suspicious/noExplicitAny: @@ -43,7 +44,7 @@ export function logWarn( export function logError( type: string, message: string, - err: Error, + err?: Error, notification?: Notification, ) { logMessage(log.error, type, message, err, notification); From c59cc2b934f54c8c8082a9388fecd31430250f39 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 30 Dec 2024 07:46:09 +0100 Subject: [PATCH 5/6] refactor: extract all loggers to helper Signed-off-by: Adam Setch --- src/main/utils.ts | 6 +++++- src/shared/logger.test.ts | 18 +++++++++--------- src/shared/logger.ts | 6 +++--- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main/utils.ts b/src/main/utils.ts index a3854d7d2..fc531ad33 100644 --- a/src/main/utils.ts +++ b/src/main/utils.ts @@ -43,7 +43,11 @@ export function openLogsDirectory() { const logDirectory = path.dirname(log.transports.file?.getFile()?.path); if (!logDirectory) { - logError('openLogsDirectory', 'Could not find log directory!'); + logError( + 'openLogsDirectory', + 'Could not find log directory!', + new Error('Directory not found'), + ); return; } diff --git a/src/shared/logger.test.ts b/src/shared/logger.test.ts index cb0b6f0a0..468741f2f 100644 --- a/src/shared/logger.test.ts +++ b/src/shared/logger.test.ts @@ -21,7 +21,7 @@ describe('renderer/utils/logger.ts', () => { logInfo('foo', 'bar'); expect(logInfoSpy).toHaveBeenCalledTimes(1); - expect(logInfoSpy).toHaveBeenCalledWith('[foo]:', 'bar'); + expect(logInfoSpy).toHaveBeenCalledWith('[foo]', 'bar'); }); it('log info with notification', () => { @@ -29,9 +29,9 @@ describe('renderer/utils/logger.ts', () => { expect(logInfoSpy).toHaveBeenCalledTimes(1); expect(logInfoSpy).toHaveBeenCalledWith( - '[foo]:', + '[foo]', 'bar', - '[Issue]: I am a robot and this is a test! for repository gitify-app/notifications-test', + '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', ); }); }); @@ -41,7 +41,7 @@ describe('renderer/utils/logger.ts', () => { logWarn('foo', 'bar'); expect(logWarnSpy).toHaveBeenCalledTimes(1); - expect(logWarnSpy).toHaveBeenCalledWith('[foo]:', 'bar'); + expect(logWarnSpy).toHaveBeenCalledWith('[foo]', 'bar'); }); it('log warn with notification', () => { @@ -49,9 +49,9 @@ describe('renderer/utils/logger.ts', () => { expect(logWarnSpy).toHaveBeenCalledTimes(1); expect(logWarnSpy).toHaveBeenCalledWith( - '[foo]:', + '[foo]', 'bar', - '[Issue]: I am a robot and this is a test! for repository gitify-app/notifications-test', + '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', ); }); }); @@ -61,7 +61,7 @@ describe('renderer/utils/logger.ts', () => { logError('foo', 'bar', mockError); expect(logErrorSpy).toHaveBeenCalledTimes(1); - expect(logErrorSpy).toHaveBeenCalledWith('[foo]:', 'bar', mockError); + expect(logErrorSpy).toHaveBeenCalledWith('[foo]', 'bar', mockError); }); it('log error with notification', () => { @@ -69,9 +69,9 @@ describe('renderer/utils/logger.ts', () => { expect(logErrorSpy).toHaveBeenCalledTimes(1); expect(logErrorSpy).toHaveBeenCalledWith( - '[foo]:', + '[foo]', 'bar', - '[Issue]: I am a robot and this is a test! for repository gitify-app/notifications-test', + '[Issue >> gitify-app/notifications-test >> I am a robot and this is a test!]', mockError, ); }); diff --git a/src/shared/logger.ts b/src/shared/logger.ts index 972e6af53..2a688146e 100644 --- a/src/shared/logger.ts +++ b/src/shared/logger.ts @@ -10,11 +10,11 @@ function logMessage( err?: Error, notification?: Notification, ) { - const args: (string | Error)[] = [`[${type}]:`, message]; + const args: (string | Error)[] = [`[${type}]`, message]; if (notification) { args.push( - `[${notification.subject.type}]: ${notification.subject.title} for repository ${notification.repository.full_name}`, + `[${notification.subject.type} >> ${notification.repository.full_name} >> ${notification.subject.title}]`, ); } @@ -44,7 +44,7 @@ export function logWarn( export function logError( type: string, message: string, - err?: Error, + err: Error, notification?: Notification, ) { logMessage(log.error, type, message, err, notification); From fbd38789a6c00f088f054c80b9593f42e2ab2240 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 31 Dec 2024 08:19:42 +0100 Subject: [PATCH 6/6] refactor: extract all loggers to helper Signed-off-by: Adam Setch --- src/shared/logger.ts | 46 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/shared/logger.ts b/src/shared/logger.ts index 2a688146e..f9242160f 100644 --- a/src/shared/logger.ts +++ b/src/shared/logger.ts @@ -2,29 +2,6 @@ import log from 'electron-log'; import type { Notification } from '../renderer/typesGitHub'; -function logMessage( - // biome-ignore lint/suspicious/noExplicitAny: - logFunction: (...params: any[]) => void, - type: string, - message: string, - err?: Error, - notification?: Notification, -) { - const args: (string | Error)[] = [`[${type}]`, message]; - - if (notification) { - args.push( - `[${notification.subject.type} >> ${notification.repository.full_name} >> ${notification.subject.title}]`, - ); - } - - if (err) { - args.push(err); - } - - logFunction(...args); -} - export function logInfo( type: string, message: string, @@ -49,3 +26,26 @@ export function logError( ) { logMessage(log.error, type, message, err, notification); } + +function logMessage( + // biome-ignore lint/suspicious/noExplicitAny: + logFunction: (...params: any[]) => void, + type: string, + message: string, + err?: Error, + notification?: Notification, +) { + const args: (string | Error)[] = [`[${type}]`, message]; + + if (notification) { + args.push( + `[${notification.subject.type} >> ${notification.repository.full_name} >> ${notification.subject.title}]`, + ); + } + + if (err) { + args.push(err); + } + + logFunction(...args); +}