Fix unhandled promise rejections and test type errors Update `getPatchContent` in `RestApi` to accept an optional error callback. Use this callback in `GrAiPromptDialog` and `GrDownloadDialog` to dispatch error events, preventing unhandled promise rejections when fetching patch content fails. Update `RestApiMock` to return `undefined` for `getChange` instead of throwing an error, preventing "Not implemented" crashes during tests. Fix TypeScript errors in `gr-comment-thread_test` by adding missing `_account_id` fields to mock data and stubbing `discardDraft`. Update related tests to align with the new error handling patterns. Release-Notes: skip Change-Id: Ib8e578a17c1204039db6fcf5acf045d9649b9402
diff --git a/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog.ts b/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog.ts index 167b388..a990898 100644 --- a/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog.ts +++ b/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog.ts
@@ -330,13 +330,11 @@ const content = await this.restApiService.getPatchContent( this.change._number, this.patchNum, - this.context + this.context, + () => fireError(this, 'Failed to get patch content') ); this.loading = false; - if (!content) { - fireError(this, 'Failed to get patch content'); - return; - } + if (!content) return; this.patchContent = content; }
diff --git a/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog_test.ts index 699035e..8165a0f 100644 --- a/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog_test.ts +++ b/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog_test.ts
@@ -9,12 +9,14 @@ import {GrAiPromptDialog} from './gr-ai-prompt-dialog'; import {createParsedChange} from '../../../test/test-data-generators'; import {CommitId, PatchSetNum} from '../../../api/rest-api'; -import {stubRestApi} from '../../../test/test-utils'; +import {stubRestApi, waitUntil} from '../../../test/test-utils'; suite('gr-ai-prompt-dialog test', () => { let element: GrAiPromptDialog; + let getPatchContentStub: sinon.SinonStub; setup(async () => { - stubRestApi('getPatchContent').returns(Promise.resolve('test code')); + getPatchContentStub = stubRestApi('getPatchContent'); + getPatchContentStub.resolves('test code'); element = await fixture(html`<gr-ai-prompt-dialog></gr-ai-prompt-dialog>`); element.change = createParsedChange(); element.change.revisions['abc'].commit!.parents = [ @@ -142,4 +144,27 @@ </section>` ); }); + + test('handles failed patch content fetch', async () => { + getPatchContentStub.callsFake((_c, _p, _ctx, errFn) => { + if (errFn) errFn(); + return Promise.resolve(undefined); + }); + const fireStub = sinon.stub(element, 'dispatchEvent'); + + element.open(); + + await waitUntil(() => fireStub.called); + + assert.isTrue(fireStub.called); + const events = fireStub.args.map(arg => arg[0]); + assert.isTrue( + events.some( + event => + event.type === 'show-error' && + (event as CustomEvent).detail.message === + 'Failed to get patch content' + ) + ); + }); });
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts index 5bccb07..95df2fac 100644 --- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts +++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
@@ -280,12 +280,11 @@ if (!this.change || !this.patchNum) return; const patchContent = await this.restApiService.getPatchContent( this.change._number, - this.patchNum + this.patchNum, + undefined, + () => fireError(this, 'Failed to get patch content') ); - if (!patchContent) { - fireError(this, 'Failed to get patch content'); - return; - } + if (!patchContent) return; await copyToClipboard(patchContent, 'patch file content'); this.handleCloseTap(e); }
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts index d71c2af..c107c20 100644 --- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts +++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
@@ -350,7 +350,10 @@ }); test('handles failed patch content fetch', async () => { - getPatchContentStub.resolves(undefined); + getPatchContentStub.callsFake((_c, _p, _ctx, errFn) => { + if (errFn) errFn(); + return Promise.resolve(undefined); + }); const copyButton = queryAndAssert<GrButton>( element, @@ -362,20 +365,5 @@ assert.isFalse(copyToClipboardStub.called); assert.isTrue(fireStub.called); }); - - test('handles error during patch content fetch', async () => { - const error = new Error('Network error'); - getPatchContentStub.rejects(error); - - const copyButton = queryAndAssert<GrButton>( - element, - '#copy-clipboard-button' - ); - copyButton.click(); - - await waitUntil(() => getPatchContentStub.called); - assert.isFalse(copyToClipboardStub.called); - assert.isFalse(fireStub.called); - }); }); });
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts index 61c23d8..165dc03 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -9,6 +9,7 @@ import {sortComments} from '../../../utils/comment-util'; import {GrCommentThread} from './gr-comment-thread'; import { + AccountId, CommentInfo, CommentThread, DraftInfo, @@ -55,14 +56,14 @@ import {suggestionsServiceToken} from '../../../services/suggestions/suggestions-service'; const c1: CommentInfo = { - author: {name: 'Kermit'}, + author: {name: 'Kermit', _account_id: 1 as AccountId}, id: 'the-root' as UrlEncodedCommentId, message: 'start the conversation', updated: '2021-11-01 10:11:12.000000000' as Timestamp, }; const c2: CommentInfo = { - author: {name: 'Ms Piggy'}, + author: {name: 'Ms Piggy', _account_id: 2 as AccountId}, id: 'the-reply' as UrlEncodedCommentId, message: 'keep it going', updated: '2021-11-02 10:11:12.000000000' as Timestamp, @@ -70,7 +71,7 @@ }; const c3: DraftInfo = { - author: {name: 'Kermit'}, + author: {name: 'Kermit', _account_id: 1 as AccountId}, id: 'the-draft' as UrlEncodedCommentId, message: 'stop it', updated: '2021-11-03 10:11:12.000000000' as Timestamp, @@ -79,7 +80,7 @@ }; const commentWithContext = { - author: {name: 'Kermit'}, + author: {name: 'Kermit', _account_id: 1 as AccountId}, id: 'the-draft' as UrlEncodedCommentId, message: 'just for context', updated: '2021-11-03 10:11:12.000000000' as Timestamp, @@ -343,6 +344,7 @@ .stub(testResolver(commentsModelToken), 'saveDraft') .returns(savePromise); stubAdd = sinon.stub(testResolver(commentsModelToken), 'addNewDraft'); + sinon.stub(testResolver(commentsModelToken), 'discardDraft'); element.thread = createThread(c1, {...c2, unresolved: true}); await element.updateComplete; @@ -707,7 +709,7 @@ ...createComment(), id: '123' as any, message: 'Test comment', - author: {name: 'Test User'}, + author: {name: 'Test User', _account_id: 12345 as AccountId}, patch_set: 1 as RevisionPatchSetNum, line: 10, path: 'test.txt',
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts index d1d6638..3398967 100644 --- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts +++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -3651,7 +3651,8 @@ async getPatchContent( changeNum: NumericChangeId, patchNum: PatchSetNum, - context?: number + context?: number, + errFn?: ErrorCallback ): Promise<string | undefined> { const url = await this._changeBaseURL(changeNum, patchNum); const params: {[key: string]: string | number} = { @@ -3665,6 +3666,7 @@ url: `${url}/patch`, params, anonymizedUrl: `${ANONYMIZED_REVISION_BASE_URL}/patch`, + errFn, }); if (!response?.ok) return undefined; return await response.text();
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts index 311dbd3..9b41a9f 100644 --- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts +++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -842,7 +842,8 @@ getPatchContent( changeNum: NumericChangeId, patchNum: PatchSetNum, - context?: number + context?: number, + errFn?: ErrorCallback ): Promise<string | undefined>; getImagesForDiff(
diff --git a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts index 400a7e6..422d00a 100644 --- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts +++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -231,7 +231,7 @@ return Promise.resolve({}); }, getChange(): Promise<ChangeInfo | undefined> { - throw new Error('getChange() not implemented by RestApiMock.'); + return Promise.resolve(undefined); }, getChangeActionURL(): Promise<string> { return Promise.resolve('');