Skip to content

Commit 037af4e

Browse files
authored
fix: handle exceptions when fetching specific html url (#1552)
Signed-off-by: Adam Setch <[email protected]>
1 parent 1f3f4af commit 037af4e

File tree

2 files changed

+87
-44
lines changed

2 files changed

+87
-44
lines changed

src/utils/helpers.test.ts

+49-14
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
mockPersonalAccessTokenAccount,
66
} from '../__mocks__/state-mocks';
77

8+
import log from 'electron-log';
89
import { defaultSettings } from '../context/App';
910
import type { Hostname, Link, SettingsState } from '../types';
1011
import type { SubjectType } from '../typesGitHub';
@@ -505,23 +506,57 @@ describe('utils/helpers.ts', () => {
505506
});
506507
});
507508

508-
it('defaults to repository url', async () => {
509-
const subject = {
510-
title: 'generate github web url unit tests',
511-
url: null,
512-
latest_comment_url: null,
513-
type: 'Issue' as SubjectType,
514-
};
509+
describe('defaults to repository url', () => {
510+
it('defaults when no urls present in notification', async () => {
511+
const subject = {
512+
title: 'generate github web url unit tests',
513+
url: null,
514+
latest_comment_url: null,
515+
type: 'Issue' as SubjectType,
516+
};
515517

516-
const result = await generateGitHubWebUrl({
517-
...mockSingleNotification,
518-
subject: subject,
518+
const result = await generateGitHubWebUrl({
519+
...mockSingleNotification,
520+
subject: subject,
521+
});
522+
523+
expect(apiRequestAuthMock).toHaveBeenCalledTimes(0);
524+
expect(result).toBe(
525+
`${mockSingleNotification.repository.html_url}?${mockNotificationReferrer}`,
526+
);
519527
});
520528

521-
expect(apiRequestAuthMock).toHaveBeenCalledTimes(0);
522-
expect(result).toBe(
523-
`${mockSingleNotification.repository.html_url}?${mockNotificationReferrer}`,
524-
);
529+
it('defaults when exception handled during specialized html enrichment process', async () => {
530+
const logErrorSpy = jest.spyOn(log, 'error').mockImplementation();
531+
532+
const subject = {
533+
title: 'generate github web url unit tests',
534+
url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/1' as Link,
535+
latest_comment_url:
536+
'https://api.github.com/repos/gitify-app/notifications-test/issues/comments/302888448' as Link,
537+
type: 'Issue' as SubjectType,
538+
};
539+
540+
const mockError = new Error('Test error');
541+
542+
apiRequestAuthMock.mockRejectedValue(mockError);
543+
544+
const result = await generateGitHubWebUrl({
545+
...mockSingleNotification,
546+
subject: subject,
547+
});
548+
549+
expect(apiRequestAuthMock).toHaveBeenCalledTimes(1);
550+
expect(apiRequestAuthMock).toHaveBeenCalledWith(
551+
subject.latest_comment_url,
552+
'GET',
553+
mockPersonalAccessTokenAccount.token,
554+
);
555+
expect(result).toBe(
556+
`https://github.com/gitify-app/notifications-test?${mockNotificationReferrer}`,
557+
);
558+
expect(logErrorSpy).toHaveBeenCalledTimes(2);
559+
});
525560
});
526561
});
527562

src/utils/helpers.ts

+38-30
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { formatDistanceToNowStrict, parseISO } from 'date-fns';
2+
import log from 'electron-log';
23
import { defaultSettings } from '../context/App';
34
import type { Account, Hostname, Link, SettingsState } from '../types';
45
import type { Notification } from '../typesGitHub';
@@ -117,37 +118,44 @@ export async function generateGitHubWebUrl(
117118
): Promise<Link> {
118119
const url = new URL(notification.repository.html_url);
119120

120-
if (notification.subject.latest_comment_url) {
121-
url.href = await getHtmlUrl(
122-
notification.subject.latest_comment_url,
123-
notification.account.token,
124-
);
125-
} else if (notification.subject.url) {
126-
url.href = await getHtmlUrl(
127-
notification.subject.url,
128-
notification.account.token,
129-
);
130-
} else {
131-
// Perform any specific notification type handling (only required for a few special notification scenarios)
132-
switch (notification.subject.type) {
133-
case 'CheckSuite':
134-
url.href = getCheckSuiteUrl(notification);
135-
break;
136-
case 'Discussion':
137-
url.href = await getDiscussionUrl(notification);
138-
break;
139-
case 'RepositoryInvitation':
140-
url.pathname += '/invitations';
141-
break;
142-
case 'RepositoryDependabotAlertsThread':
143-
url.pathname += '/security/dependabot';
144-
break;
145-
case 'WorkflowRun':
146-
url.href = getWorkflowRunUrl(notification);
147-
break;
148-
default:
149-
break;
121+
try {
122+
if (notification.subject.latest_comment_url) {
123+
url.href = await getHtmlUrl(
124+
notification.subject.latest_comment_url,
125+
notification.account.token,
126+
);
127+
} else if (notification.subject.url) {
128+
url.href = await getHtmlUrl(
129+
notification.subject.url,
130+
notification.account.token,
131+
);
132+
} else {
133+
// Perform any specific notification type handling (only required for a few special notification scenarios)
134+
switch (notification.subject.type) {
135+
case 'CheckSuite':
136+
url.href = getCheckSuiteUrl(notification);
137+
break;
138+
case 'Discussion':
139+
url.href = await getDiscussionUrl(notification);
140+
break;
141+
case 'RepositoryInvitation':
142+
url.pathname += '/invitations';
143+
break;
144+
case 'RepositoryDependabotAlertsThread':
145+
url.pathname += '/security/dependabot';
146+
break;
147+
case 'WorkflowRun':
148+
url.href = getWorkflowRunUrl(notification);
149+
break;
150+
default:
151+
break;
152+
}
150153
}
154+
} catch (err) {
155+
log.error(
156+
'Error occurred while attempting to get a specific notification URL. Will fall back to defaults',
157+
err,
158+
);
151159
}
152160

153161
url.searchParams.set(

0 commit comments

Comments
 (0)