From 476de3d3a5960d833e418cfa9934e0a19ae43416 Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 26 Dec 2024 18:38:31 +0000 Subject: [PATCH 1/4] Fix issue #5831: [Bug]: "Request failed with status code 409" on opening empty repo --- frontend/__tests__/api/github.test.ts | 44 +++++++++++++++++++++++++++ frontend/public/mockServiceWorker.js | 18 +++++++++-- frontend/src/api/github.ts | 27 ++++++++++------ 3 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 frontend/__tests__/api/github.test.ts diff --git a/frontend/__tests__/api/github.test.ts b/frontend/__tests__/api/github.test.ts new file mode 100644 index 000000000000..f68bd8f14a6a --- /dev/null +++ b/frontend/__tests__/api/github.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it, vi } from 'vitest'; +import { github } from '../../src/api/github-axios-instance'; +import { retrieveLatestGitHubCommit } from '../../src/api/github'; + +vi.mock('../../src/api/github-axios-instance', () => ({ + github: { + get: vi.fn(), + }, +})); + +describe('retrieveLatestGitHubCommit', () => { + it('should return the latest commit when repository has commits', async () => { + const mockCommit = { + sha: '123abc', + commit: { + message: 'Initial commit', + }, + }; + + (github.get as any).mockResolvedValueOnce({ + data: [mockCommit], + }); + + const result = await retrieveLatestGitHubCommit('user/repo'); + expect(result).toEqual(mockCommit); + }); + + it('should return null when repository is empty', async () => { + const error = new Error('Repository is empty'); + (error as any).response = { status: 409 }; + (github.get as any).mockRejectedValueOnce(error); + + const result = await retrieveLatestGitHubCommit('user/empty-repo'); + expect(result).toBeNull(); + }); + + it('should throw error for other error cases', async () => { + const error = new Error('Network error'); + (error as any).response = { status: 500 }; + (github.get as any).mockRejectedValueOnce(error); + + await expect(retrieveLatestGitHubCommit('user/repo')).rejects.toThrow(); + }); +}); diff --git a/frontend/public/mockServiceWorker.js b/frontend/public/mockServiceWorker.js index fead0b3ff91c..ec47a9a50a24 100644 --- a/frontend/public/mockServiceWorker.js +++ b/frontend/public/mockServiceWorker.js @@ -8,8 +8,8 @@ * - Please do NOT serve this file on production. */ -const PACKAGE_VERSION = '2.6.6' -const INTEGRITY_CHECKSUM = 'ca7800994cc8bfb5eb961e037c877074' +const PACKAGE_VERSION = '2.7.0' +const INTEGRITY_CHECKSUM = '00729d72e3b82faf54ca8b9621dbb96f' const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') const activeClientIds = new Set() @@ -199,7 +199,19 @@ async function getResponse(event, client, requestId) { // Remove the "accept" header value that marked this request as passthrough. // This prevents request alteration and also keeps it compliant with the // user-defined CORS policies. - headers.delete('accept', 'msw/passthrough') + const acceptHeader = headers.get('accept') + if (acceptHeader) { + const values = acceptHeader.split(',').map((value) => value.trim()) + const filteredValues = values.filter( + (value) => value !== 'msw/passthrough', + ) + + if (filteredValues.length > 0) { + headers.set('accept', filteredValues.join(', ')) + } else { + headers.delete('accept') + } + } return fetch(requestClone, { headers }) } diff --git a/frontend/src/api/github.ts b/frontend/src/api/github.ts index b315e2d930a7..1db017dbed5f 100644 --- a/frontend/src/api/github.ts +++ b/frontend/src/api/github.ts @@ -106,15 +106,22 @@ export const retrieveGitHubUser = async () => { export const retrieveLatestGitHubCommit = async ( repository: string, -): Promise => { - const response = await github.get( - `/repos/${repository}/commits`, - { - params: { - per_page: 1, +): Promise => { + try { + const response = await github.get( + `/repos/${repository}/commits`, + { + params: { + per_page: 1, + }, }, - }, - ); - - return response.data[0]; + ); + return response.data[0] || null; + } catch (error: any) { + if (error.response?.status === 409) { + // Repository is empty, no commits yet + return null; + } + throw error; + } }; From 81a441c937bb673cc383a047e925c9359e468b37 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 27 Dec 2024 03:58:41 +0000 Subject: [PATCH 2/4] Fix frontend linting issues --- frontend/src/api/github.ts | 12 +++++++++--- frontend/src/services/settings.ts | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/frontend/src/api/github.ts b/frontend/src/api/github.ts index 1db017dbed5f..492955ae69d9 100644 --- a/frontend/src/api/github.ts +++ b/frontend/src/api/github.ts @@ -117,11 +117,17 @@ export const retrieveLatestGitHubCommit = async ( }, ); return response.data[0] || null; - } catch (error: any) { - if (error.response?.status === 409) { + } catch (error) { + if (!error || typeof error !== "object") { + throw new Error("Unknown error occurred"); + } + const axiosError = error as { response?: { status: number } }; + if (axiosError.response?.status === 409) { // Repository is empty, no commits yet return null; } - throw error; + throw new Error( + error instanceof Error ? error.message : "Unknown error occurred", + ); } }; diff --git a/frontend/src/services/settings.ts b/frontend/src/services/settings.ts index ceed34b4a17f..ef3061705cfe 100644 --- a/frontend/src/services/settings.ts +++ b/frontend/src/services/settings.ts @@ -133,7 +133,7 @@ export const saveSettings = async ( const { data } = await openHands.post("/api/settings", apiSettings); return data; } catch (error) { - console.error("Error saving settings:", error); + // Error handled by returning false return false; } }; From 64c9c949ea6f1865be26d002f6472dcd3c570fb2 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 30 Dec 2024 01:15:30 +0000 Subject: [PATCH 3/4] Fix pr #5833: Fix issue #5831: [Bug]: "Request failed with status code 409" on opening empty repo --- frontend/public/mockServiceWorker.js | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/frontend/public/mockServiceWorker.js b/frontend/public/mockServiceWorker.js index ec47a9a50a24..fead0b3ff91c 100644 --- a/frontend/public/mockServiceWorker.js +++ b/frontend/public/mockServiceWorker.js @@ -8,8 +8,8 @@ * - Please do NOT serve this file on production. */ -const PACKAGE_VERSION = '2.7.0' -const INTEGRITY_CHECKSUM = '00729d72e3b82faf54ca8b9621dbb96f' +const PACKAGE_VERSION = '2.6.6' +const INTEGRITY_CHECKSUM = 'ca7800994cc8bfb5eb961e037c877074' const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') const activeClientIds = new Set() @@ -199,19 +199,7 @@ async function getResponse(event, client, requestId) { // Remove the "accept" header value that marked this request as passthrough. // This prevents request alteration and also keeps it compliant with the // user-defined CORS policies. - const acceptHeader = headers.get('accept') - if (acceptHeader) { - const values = acceptHeader.split(',').map((value) => value.trim()) - const filteredValues = values.filter( - (value) => value !== 'msw/passthrough', - ) - - if (filteredValues.length > 0) { - headers.set('accept', filteredValues.join(', ')) - } else { - headers.delete('accept') - } - } + headers.delete('accept', 'msw/passthrough') return fetch(requestClone, { headers }) } From a37ec90e871524fa572c52b80233f3e7a17228ea Mon Sep 17 00:00:00 2001 From: amanape <83104063+amanape@users.noreply.github.com> Date: Mon, 30 Dec 2024 15:55:47 +0400 Subject: [PATCH 4/4] Lint and improve test --- frontend/__tests__/api/github.test.ts | 51 ++++++++++++++------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/frontend/__tests__/api/github.test.ts b/frontend/__tests__/api/github.test.ts index f68bd8f14a6a..5a659d4b71e6 100644 --- a/frontend/__tests__/api/github.test.ts +++ b/frontend/__tests__/api/github.test.ts @@ -1,44 +1,47 @@ -import { describe, expect, it, vi } from 'vitest'; -import { github } from '../../src/api/github-axios-instance'; -import { retrieveLatestGitHubCommit } from '../../src/api/github'; - -vi.mock('../../src/api/github-axios-instance', () => ({ - github: { - get: vi.fn(), - }, -})); - -describe('retrieveLatestGitHubCommit', () => { - it('should return the latest commit when repository has commits', async () => { +import { describe, expect, it, vi } from "vitest"; +import { retrieveLatestGitHubCommit } from "../../src/api/github"; + +describe("retrieveLatestGitHubCommit", () => { + const { githubGetMock } = vi.hoisted(() => ({ + githubGetMock: vi.fn(), + })); + + vi.mock("../../src/api/github-axios-instance", () => ({ + github: { + get: githubGetMock, + }, + })); + + it("should return the latest commit when repository has commits", async () => { const mockCommit = { - sha: '123abc', + sha: "123abc", commit: { - message: 'Initial commit', + message: "Initial commit", }, }; - (github.get as any).mockResolvedValueOnce({ + githubGetMock.mockResolvedValueOnce({ data: [mockCommit], }); - const result = await retrieveLatestGitHubCommit('user/repo'); + const result = await retrieveLatestGitHubCommit("user/repo"); expect(result).toEqual(mockCommit); }); - it('should return null when repository is empty', async () => { - const error = new Error('Repository is empty'); + it("should return null when repository is empty", async () => { + const error = new Error("Repository is empty"); (error as any).response = { status: 409 }; - (github.get as any).mockRejectedValueOnce(error); + githubGetMock.mockRejectedValueOnce(error); - const result = await retrieveLatestGitHubCommit('user/empty-repo'); + const result = await retrieveLatestGitHubCommit("user/empty-repo"); expect(result).toBeNull(); }); - it('should throw error for other error cases', async () => { - const error = new Error('Network error'); + it("should throw error for other error cases", async () => { + const error = new Error("Network error"); (error as any).response = { status: 500 }; - (github.get as any).mockRejectedValueOnce(error); + githubGetMock.mockRejectedValueOnce(error); - await expect(retrieveLatestGitHubCommit('user/repo')).rejects.toThrow(); + await expect(retrieveLatestGitHubCommit("user/repo")).rejects.toThrow(); }); });