Skip to content

Commit

Permalink
feat: improve error logging (#1455)
Browse files Browse the repository at this point in the history
Signed-off-by: Adam Setch <adam.setch@outlook.com>
  • Loading branch information
setchy authored Aug 12, 2024
1 parent cafd2e6 commit 4edc4be
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 14 deletions.
10 changes: 10 additions & 0 deletions src/hooks/useNotifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +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 {
mockAuth,
mockGitHubCloudAccount,
Expand All @@ -16,10 +17,13 @@ import { Errors } from '../utils/constants';
import { useNotifications } from './useNotifications';

describe('hooks/useNotifications.ts', () => {
const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();

beforeEach(() => {
// axios will default to using the XHR adapter which can't be intercepted
// by nock. So, configure axios to use the node adapter.
axios.defaults.adapter = 'http';
logErrorSpy.mockReset();
});

const id = mockSingleNotification.id;
Expand Down Expand Up @@ -294,6 +298,7 @@ describe('hooks/useNotifications.ts', () => {
});

expect(result.current.globalError).toBe(Errors.BAD_CREDENTIALS);
expect(logErrorSpy).toHaveBeenCalledTimes(2);
});

it('should fetch notifications with different failures', async () => {
Expand Down Expand Up @@ -336,6 +341,7 @@ describe('hooks/useNotifications.ts', () => {
});

expect(result.current.globalError).toBeNull();
expect(logErrorSpy).toHaveBeenCalledTimes(2);
});
});

Expand Down Expand Up @@ -374,6 +380,7 @@ describe('hooks/useNotifications.ts', () => {
});

expect(result.current.notifications.length).toBe(0);
expect(logErrorSpy).toHaveBeenCalledTimes(1);
});
});

Expand Down Expand Up @@ -412,6 +419,7 @@ describe('hooks/useNotifications.ts', () => {
});

expect(result.current.notifications.length).toBe(0);
expect(logErrorSpy).toHaveBeenCalledTimes(1);
});
});

Expand Down Expand Up @@ -470,6 +478,7 @@ describe('hooks/useNotifications.ts', () => {
});

expect(result.current.notifications.length).toBe(0);
expect(logErrorSpy).toHaveBeenCalledTimes(1);
});
});

Expand Down Expand Up @@ -516,6 +525,7 @@ describe('hooks/useNotifications.ts', () => {
});

expect(result.current.notifications.length).toBe(0);
expect(logErrorSpy).toHaveBeenCalledTimes(1);
});
});

Expand Down
35 changes: 25 additions & 10 deletions src/hooks/useNotifications.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import log from 'electron-log';
import { useCallback, useState } from 'react';
import type {
AccountNotifications,
Expand Down Expand Up @@ -109,10 +110,11 @@ export const useNotifications = (): NotificationsState => {

setNotifications(updatedNotifications);
setTrayIconColor(updatedNotifications);
setStatus('success');
} catch (err) {
setStatus('success');
log.error('Error occurred while marking notification as read', err);
}

setStatus('success');
},
[notifications],
);
Expand All @@ -138,10 +140,11 @@ export const useNotifications = (): NotificationsState => {

setNotifications(updatedNotifications);
setTrayIconColor(updatedNotifications);
setStatus('success');
} catch (err) {
setStatus('success');
log.error('Error occurred while marking notification as done', err);
}

setStatus('success');
},
[notifications],
);
Expand All @@ -157,10 +160,14 @@ export const useNotifications = (): NotificationsState => {
notification.account.token,
);
await markNotificationRead(state, notification);
setStatus('success');
} catch (err) {
setStatus('success');
log.error(
'Error occurred while unsubscribing from notification thread',
err,
);
}

setStatus('success');
},
[markNotificationRead],
);
Expand All @@ -187,10 +194,14 @@ export const useNotifications = (): NotificationsState => {

setNotifications(updatedNotifications);
setTrayIconColor(updatedNotifications);
setStatus('success');
} catch (err) {
setStatus('success');
log.error(
'Error occurred while marking repository notifications as read',
err,
);
}

setStatus('success');
},
[notifications],
);
Expand Down Expand Up @@ -230,10 +241,14 @@ export const useNotifications = (): NotificationsState => {

setNotifications(updatedNotifications);
setTrayIconColor(updatedNotifications);
setStatus('success');
} catch (err) {
setStatus('success');
log.error(
'Error occurred while marking repository notifications as done',
err,
);
}

setStatus('success');
},
[notifications, markNotificationDone],
);
Expand Down
2 changes: 1 addition & 1 deletion src/routes/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const LoginRoute: FC = () => {
try {
await login();
} catch (err) {
// Skip
log.error('Auth: failed to login with GitHub', err);
}
}, []); */

Expand Down
2 changes: 2 additions & 0 deletions src/routes/LoginWithOAuthApp.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
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 { Header } from '../components/Header';
Expand Down Expand Up @@ -120,6 +121,7 @@ export const LoginWithOAuthApp: FC = () => {
try {
await loginWithOAuthApp(data as LoginOAuthAppOptions);
} catch (err) {
log.error('Auth: Failed to login with oauth app', err);
// Skip
}
},
Expand Down
2 changes: 2 additions & 0 deletions src/routes/LoginWithPersonalAccessToken.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
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';
Expand Down Expand Up @@ -125,6 +126,7 @@ export const LoginWithPersonalAccessToken: FC = () => {
);
navigate(-1);
} catch (err) {
log.error('Auth: failed to login with personal access token', err);
setIsValidToken(false);
}
},
Expand Down
2 changes: 1 addition & 1 deletion src/utils/api/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ describe('utils/api/client.ts', () => {
'123' as Token,
);

expect(logErrorSpy).toHaveBeenCalledWith('Failed to get html url');
expect(logErrorSpy).toHaveBeenCalledTimes(1);
});
});
});
9 changes: 7 additions & 2 deletions src/utils/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ 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('Failed to get html url');
log.error('Error occurred while fetching notification html url', err);
}
}

Expand Down Expand Up @@ -272,5 +272,10 @@ export async function getLatestDiscussion(
(discussion) => discussion.title === notification.subject.title,
)[0] ?? null
);
} catch (err) {}
} catch (err) {
log.error(
'Error occurred while fetching notification latest discussion',
err,
);
}
}
5 changes: 5 additions & 0 deletions src/utils/notifications.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import log from 'electron-log';
import type {
AccountNotifications,
GitifyState,
Expand Down Expand Up @@ -148,6 +149,10 @@ export async function getAllNotifications(
error: null,
};
} catch (error) {
log.error(
'Error occurred while fetching account notifications',
error,
);
return {
account: accountNotifications.account,
notifications: [],
Expand Down

0 comments on commit 4edc4be

Please sign in to comment.