Skip to content

Commit dd18836

Browse files
authored
refactor: render logger utility (#2181)
Signed-off-by: Adam Setch <adam.setch@outlook.com>
1 parent ce3ced8 commit dd18836

21 files changed

+245
-102
lines changed

src/renderer/__mocks__/notifications-mocks.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { StateType, Subject, SubjectType } from '../typesGitHub';
33
import {
44
mockEnterpriseNotifications,
55
mockGitHubNotifications,
6+
mockSingleNotification,
67
} from '../utils/api/__mocks__/response-mocks';
78
import {
89
mockGitHubCloudAccount,
@@ -25,7 +26,7 @@ export const mockAccountNotifications: AccountNotifications[] = [
2526
export const mockSingleAccountNotifications: AccountNotifications[] = [
2627
{
2728
account: mockGitHubCloudAccount,
28-
notifications: [mockGitHubNotifications[0]],
29+
notifications: [mockSingleNotification],
2930
error: null,
3031
},
3132
];

src/renderer/hooks/useNotifications.test.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { act, renderHook, waitFor } from '@testing-library/react';
33
import axios, { AxiosError } from 'axios';
44
import nock from 'nock';
55

6-
import * as logger from '../../shared/logger';
7-
86
import {
97
mockAuth,
108
mockGitHubCloudAccount,
@@ -16,16 +14,19 @@ import {
1614
mockSingleNotification,
1715
} from '../utils/api/__mocks__/response-mocks';
1816
import { Errors } from '../utils/errors';
17+
import * as logger from '../utils/logger';
1918
import { useNotifications } from './useNotifications';
2019

2120
describe('renderer/hooks/useNotifications.ts', () => {
22-
const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation();
21+
const rendererLogErrorSpy = jest
22+
.spyOn(logger, 'rendererLogError')
23+
.mockImplementation();
2324

2425
beforeEach(() => {
2526
// axios will default to using the XHR adapter which can't be intercepted
2627
// by nock. So, configure axios to use the node adapter.
2728
axios.defaults.adapter = 'http';
28-
logErrorSpy.mockReset();
29+
rendererLogErrorSpy.mockReset();
2930
});
3031

3132
const id = mockSingleNotification.id;
@@ -300,7 +301,7 @@ describe('renderer/hooks/useNotifications.ts', () => {
300301
});
301302

302303
expect(result.current.globalError).toBe(Errors.BAD_CREDENTIALS);
303-
expect(logErrorSpy).toHaveBeenCalledTimes(4);
304+
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(4);
304305
});
305306

306307
it('should fetch notifications with different failures', async () => {
@@ -343,7 +344,7 @@ describe('renderer/hooks/useNotifications.ts', () => {
343344
});
344345

345346
expect(result.current.globalError).toBeNull();
346-
expect(logErrorSpy).toHaveBeenCalledTimes(4);
347+
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(4);
347348
});
348349
});
349350

@@ -386,7 +387,7 @@ describe('renderer/hooks/useNotifications.ts', () => {
386387
});
387388

388389
expect(result.current.notifications.length).toBe(0);
389-
expect(logErrorSpy).toHaveBeenCalledTimes(1);
390+
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1);
390391
});
391392
});
392393

@@ -429,7 +430,7 @@ describe('renderer/hooks/useNotifications.ts', () => {
429430
});
430431

431432
expect(result.current.notifications.length).toBe(0);
432-
expect(logErrorSpy).toHaveBeenCalledTimes(1);
433+
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1);
433434
});
434435
});
435436

@@ -521,7 +522,7 @@ describe('renderer/hooks/useNotifications.ts', () => {
521522
});
522523

523524
expect(result.current.notifications.length).toBe(0);
524-
expect(logErrorSpy).toHaveBeenCalledTimes(1);
525+
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1);
525526
});
526527
});
527528
});

src/renderer/hooks/useNotifications.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import { useCallback, useState } from 'react';
22

3-
import { logError } from '../../shared/logger';
4-
53
import type {
64
Account,
75
AccountNotifications,
@@ -17,6 +15,7 @@ import {
1715
} from '../utils/api/client';
1816
import { updateTrayIcon } from '../utils/comms';
1917
import { isMarkAsDoneFeatureSupported } from '../utils/features';
18+
import { rendererLogError } from '../utils/logger';
2019
import { triggerNativeNotifications } from '../utils/notifications/native';
2120
import {
2221
getAllNotifications,
@@ -129,7 +128,7 @@ export const useNotifications = (): NotificationsState => {
129128
setNotifications(updatedNotifications);
130129
setTrayIconColor(updatedNotifications);
131130
} catch (err) {
132-
logError(
131+
rendererLogError(
133132
'markNotificationsAsRead',
134133
'Error occurred while marking notifications as read',
135134
err,
@@ -167,7 +166,7 @@ export const useNotifications = (): NotificationsState => {
167166
setNotifications(updatedNotifications);
168167
setTrayIconColor(updatedNotifications);
169168
} catch (err) {
170-
logError(
169+
rendererLogError(
171170
'markNotificationsAsDone',
172171
'Error occurred while marking notifications as done',
173172
err,
@@ -196,7 +195,7 @@ export const useNotifications = (): NotificationsState => {
196195
await markNotificationsAsRead(state, [notification]);
197196
}
198197
} catch (err) {
199-
logError(
198+
rendererLogError(
200199
'unsubscribeNotification',
201200
'Error occurred while unsubscribing from notification thread',
202201
err,

src/renderer/routes/Accounts.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ import {
2222
Text,
2323
} from '@primer/react';
2424

25-
import { logError } from '../../shared/logger';
26-
2725
import { AvatarWithFallback } from '../components/avatars/AvatarWithFallback';
2826
import { Contents } from '../components/layout/Contents';
2927
import { Page } from '../components/layout/Page';
@@ -44,6 +42,7 @@ import {
4442
openDeveloperSettings,
4543
openHost,
4644
} from '../utils/links';
45+
import { rendererLogError } from '../utils/logger';
4746
import { saveState } from '../utils/storage';
4847

4948
export const AccountsRoute: FC = () => {
@@ -98,7 +97,7 @@ export const AccountsRoute: FC = () => {
9897
try {
9998
await loginWithGitHubApp();
10099
} catch (err) {
101-
logError('loginWithGitHub', 'failed to login with GitHub', err);
100+
rendererLogError('loginWithGitHub', 'failed to login with GitHub', err);
102101
}
103102
}, []);
104103

src/renderer/routes/Login.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ import { useNavigate } from 'react-router-dom';
44
import { KeyIcon, MarkGithubIcon, PersonIcon } from '@primer/octicons-react';
55
import { Button, Heading, Stack, Text } from '@primer/react';
66

7-
import { logError } from '../../shared/logger';
8-
97
import { LogoIcon } from '../components/icons/LogoIcon';
108
import { Centered } from '../components/layout/Centered';
119
import { AppContext } from '../context/App';
1210
import { Size } from '../types';
1311
import { showWindow } from '../utils/comms';
12+
import { rendererLogError } from '../utils/logger';
1413

1514
export const LoginRoute: FC = () => {
1615
const navigate = useNavigate();
@@ -27,7 +26,11 @@ export const LoginRoute: FC = () => {
2726
try {
2827
await loginWithGitHubApp();
2928
} catch (err) {
30-
logError('loginWithGitHubApp', 'failed to login with GitHub', err);
29+
rendererLogError(
30+
'loginWithGitHubApp',
31+
'failed to login with GitHub',
32+
err,
33+
);
3134
}
3235
}, [loginWithGitHubApp]);
3336

src/renderer/routes/LoginWithOAuthApp.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import {
1818
} from '@primer/react';
1919
import { Banner } from '@primer/react/experimental';
2020

21-
import { logError } from '../../shared/logger';
22-
2321
import { Contents } from '../components/layout/Contents';
2422
import { Page } from '../components/layout/Page';
2523
import { Footer } from '../components/primitives/Footer';
@@ -35,6 +33,7 @@ import {
3533
} from '../utils/auth/utils';
3634
import { openExternalLink } from '../utils/comms';
3735
import { Constants } from '../utils/constants';
36+
import { rendererLogError } from '../utils/logger';
3837

3938
interface IFormData {
4039
hostname?: Hostname;
@@ -120,7 +119,11 @@ export const LoginWithOAuthAppRoute: FC = () => {
120119
await loginWithOAuthApp(data as LoginOAuthAppOptions);
121120
navigate(-1);
122121
} catch (err) {
123-
logError('loginWithOAuthApp', 'Failed to login with OAuth App', err);
122+
rendererLogError(
123+
'loginWithOAuthApp',
124+
'Failed to login with OAuth App',
125+
err,
126+
);
124127
setErrors({
125128
invalidCredentialsForHost: `Failed to validate provided Client ID and Secret against ${data.hostname}`,
126129
});

src/renderer/routes/LoginWithPersonalAccessToken.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import {
1818
} from '@primer/react';
1919
import { Banner } from '@primer/react/experimental';
2020

21-
import { logError } from '../../shared/logger';
22-
2321
import { Contents } from '../components/layout/Contents';
2422
import { Page } from '../components/layout/Page';
2523
import { Footer } from '../components/primitives/Footer';
@@ -35,6 +33,7 @@ import {
3533
} from '../utils/auth/utils';
3634
import { openExternalLink } from '../utils/comms';
3735
import { Constants } from '../utils/constants';
36+
import { rendererLogError } from '../utils/logger';
3837

3938
interface IFormData {
4039
token?: Token;
@@ -112,7 +111,7 @@ export const LoginWithPersonalAccessTokenRoute: FC = () => {
112111
);
113112
navigate(-1);
114113
} catch (err) {
115-
logError(
114+
rendererLogError(
116115
'loginWithPersonalAccessToken',
117116
'Failed to login with PAT',
118117
err,

src/renderer/utils/api/client.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import axios, { type AxiosPromise, type AxiosResponse } from 'axios';
22

3-
import * as logger from '../../../shared/logger';
4-
53
import {
64
mockGitHubCloudAccount,
75
mockGitHubEnterpriseServerAccount,
86
mockToken,
97
} from '../../__mocks__/state-mocks';
108
import type { Hostname, Link, SettingsState, Token } from '../../types';
9+
import * as logger from '../../utils/logger';
1110
import {
1211
getAuthenticatedUser,
1312
getHtmlUrl,
@@ -322,7 +321,9 @@ describe('renderer/utils/api/client.ts', () => {
322321
});
323322

324323
it('should handle error', async () => {
325-
const logErrorSpy = jest.spyOn(logger, 'logError').mockImplementation();
324+
const rendererLogErrorSpy = jest
325+
.spyOn(logger, 'rendererLogError')
326+
.mockImplementation();
326327

327328
const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth');
328329

@@ -335,7 +336,7 @@ describe('renderer/utils/api/client.ts', () => {
335336
'123' as Token,
336337
);
337338

338-
expect(logErrorSpy).toHaveBeenCalledTimes(1);
339+
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1);
339340
});
340341
});
341342
});

src/renderer/utils/api/client.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import type { AxiosPromise } from 'axios';
22
import { print } from 'graphql/language/printer';
33

4-
import { logError } from '../../../shared/logger';
5-
64
import type {
75
Account,
86
Hostname,
@@ -25,6 +23,7 @@ import type {
2523
UserDetails,
2624
} from '../../typesGitHub';
2725
import { isAnsweredDiscussionFeatureSupported } from '../features';
26+
import { rendererLogError } from '../logger';
2827
import { QUERY_SEARCH_DISCUSSIONS } from './graphql/discussions';
2928
import { formatAsGitHubSearchSyntax } from './graphql/utils';
3029
import { apiRequestAuth } from './request';
@@ -219,7 +218,7 @@ export async function getHtmlUrl(url: Link, token: Token): Promise<string> {
219218
const response = (await apiRequestAuth(url, 'GET', token)).data;
220219
return response.html_url;
221220
} catch (err) {
222-
logError(
221+
rendererLogError(
223222
'getHtmlUrl',
224223
`error occurred while fetching html url for ${url}`,
225224
err,
@@ -274,7 +273,7 @@ export async function getLatestDiscussion(
274273
)[0] ?? null
275274
);
276275
} catch (err) {
277-
logError(
276+
rendererLogError(
278277
'getLatestDiscussion',
279278
'failed to fetch latest discussion for notification',
280279
err,

src/renderer/utils/api/request.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ import axios, {
44
type Method,
55
} from 'axios';
66

7-
import { logError } from '../../../shared/logger';
8-
97
import type { Link, Token } from '../../types';
108
import { decryptValue } from '../comms';
9+
import { rendererLogError } from '../logger';
1110
import { getNextURLFromLinkHeader } from './utils';
1211

1312
/**
@@ -70,7 +69,7 @@ export async function apiRequestAuth(
7069
nextUrl = getNextURLFromLinkHeader(response);
7170
}
7271
} catch (err) {
73-
logError('apiRequestAuth', 'API request failed:', err);
72+
rendererLogError('apiRequestAuth', 'API request failed:', err);
7473

7574
throw err;
7675
}

0 commit comments

Comments
 (0)