Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(shared): create electron-log wrapper #1692

Merged
merged 7 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/main/first-run.ts
Original file line number Diff line number Diff line change
@@ -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()) {
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 8 additions & 6 deletions src/main/updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
});
}
Expand Down
10 changes: 8 additions & 2 deletions src/main/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ 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, '-');

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}`),
);
});
}
Expand Down Expand Up @@ -41,7 +43,11 @@ export function openLogsDirectory() {
const logDirectory = path.dirname(log.transports.file?.getFile()?.path);

if (!logDirectory) {
log.error('Could not find log directory!');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it helped with consistently immediately, too :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely! Great suggestion

logError(
'openLogsDirectory',
'Could not find log directory!',
new Error('Directory not found'),
);
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/renderer/hooks/useNotifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { act, renderHook, waitFor } from '@testing-library/react';
import axios, { AxiosError } from 'axios';
import nock from 'nock';

import log from 'electron-log';
import * as logger from '../../shared/logger';
import {
mockAuth,
mockGitHubCloudAccount,
Expand All @@ -17,7 +17,7 @@ import { Errors } from '../utils/errors';
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
Expand Down
19 changes: 12 additions & 7 deletions src/renderer/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import log from 'electron-log';
import { useCallback, useState } from 'react';

import { logError } from '../../shared/logger';
import type {
Account,
AccountNotifications,
Expand Down Expand Up @@ -121,8 +122,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,
);
}
Expand Down Expand Up @@ -158,8 +160,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,
);
}
Expand All @@ -186,9 +189,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,
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/renderer/routes/Accounts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import {
StarIcon,
SyncIcon,
} from '@primer/octicons-react';

import log from 'electron-log';
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';
Expand Down Expand Up @@ -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);
}
}, []);

Expand Down
5 changes: 3 additions & 2 deletions src/renderer/routes/Login.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
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 { logError } from '../../shared/logger';
import { Button } from '../components/buttons/Button';
import { LogoIcon } from '../components/icons/LogoIcon';
import { AppContext } from '../context/App';
Expand All @@ -23,7 +24,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]);

Expand Down
5 changes: 3 additions & 2 deletions src/renderer/routes/LoginWithOAuthApp.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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';

import { logError } from '../../shared/logger';
import { Header } from '../components/Header';
import { Button } from '../components/buttons/Button';
import { FieldInput } from '../components/fields/FieldInput';
Expand Down Expand Up @@ -131,7 +132,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],
Expand Down
9 changes: 7 additions & 2 deletions src/renderer/routes/LoginWithPersonalAccessToken.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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';

import { logError } from '../../shared/logger';
import { Header } from '../components/Header';
import { Button } from '../components/buttons/Button';
import { FieldInput } from '../components/fields/FieldInput';
Expand Down Expand Up @@ -129,7 +130,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);
}
},
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/utils/api/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import axios, { type AxiosPromise, type AxiosResponse } from 'axios';
import log from 'electron-log';
import * as logger from '../../../shared/logger';
import {
mockGitHubCloudAccount,
mockGitHubEnterpriseServerAccount,
Expand Down Expand Up @@ -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');

Expand Down
16 changes: 9 additions & 7 deletions src/renderer/utils/api/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AxiosPromise } from 'axios';
import log from 'electron-log';
import { print } from 'graphql/language/printer';

import { logError } from '../../../shared/logger';
import type {
Account,
Hostname,
Expand Down Expand Up @@ -217,9 +218,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,
);
}
Expand Down Expand Up @@ -271,10 +272,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,
);
}
}
6 changes: 4 additions & 2 deletions src/renderer/utils/api/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import axios, {
type AxiosPromise,
type Method,
} from 'axios';
import log from 'electron-log';

import { logError } from '../../../shared/logger';
import type { Link, Token } from '../../types';
import { getNextURLFromLinkHeader } from './utils';

Expand Down Expand Up @@ -73,7 +74,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;
}

Expand Down
13 changes: 10 additions & 3 deletions src/renderer/utils/auth/migration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import log from 'electron-log';
import { logInfo } from '../../../shared/logger';
import type { Account, AuthState } from '../../types';
import { Constants } from '../constants';
import { loadState, saveState } from '../storage';
Expand All @@ -16,15 +16,22 @@ 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);

saveState({
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 {
Expand Down
9 changes: 5 additions & 4 deletions src/renderer/utils/auth/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { BrowserWindow } from '@electron/remote';
import { format } from 'date-fns';
import log from 'electron-log';
import semver from 'semver';

import { logError } from '../../../shared/logger';
import type {
Account,
AuthCode,
Expand Down Expand Up @@ -178,9 +179,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,
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/utils/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
ChevronLeftIcon,
ChevronRightIcon,
} from '@primer/octicons-react';
import log from 'electron-log';
import * as logger from '../../shared/logger';
import { defaultSettings } from '../context/App';
import type { Hostname, Link, SettingsState } from '../types';
import type { SubjectType } from '../typesGitHub';
Expand Down Expand Up @@ -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',
Expand Down
Loading
Loading