diff --git a/src/services/git-service-base.ts b/src/services/git-service-base.ts index 84f4eca..7402da0 100644 --- a/src/services/git-service-base.ts +++ b/src/services/git-service-base.ts @@ -42,6 +42,7 @@ export abstract class BaseGitService { * Safely wraps requestUrl to handle potential throws from Obsidian and provide better error messages. */ protected async safeRequest(url: string, method: string, body?: unknown, extraHeaders?: Record, silent = false): Promise { + let response: RequestUrlResponse; try { const headers: Record = { ...extraHeaders, @@ -57,20 +58,28 @@ export abstract class BaseGitService { throw: false }; - const response = await requestUrl(options); - - if (response.status >= 400) { - const errorMsg = this.parseErrorResponse(response); - if (!silent) logger.error(`Git Service Request Failed (${response.status}): ${url}`, errorMsg); - throw new Error(`Git Service Error (${response.status}): ${errorMsg}`); - } - - return response; + response = await requestUrl(options); } catch (error) { + // Network-level failure (DNS, offline, TLS, etc.) if (!silent) logger.error('Git Service Request Failed:', error); if (error instanceof Error) throw error; throw new Error(`Network error or unexpected failure: ${String(error)}`); } + + if (response.status >= 400) { + const errorMsg = this.parseErrorResponse(response); + // 404 is routinely an expected "does not exist" probe (getFile treats + // it as an empty file, gitignore lookups ignore it). Log it at debug + // level so it doesn't surface as a failure, but still throw so callers + // can handle it. Other statuses are genuine errors. + if (!silent) { + if (response.status === 404) logger.debug(`Git Service 404 (not found): ${url}`); + else logger.error(`Git Service Request Failed (${response.status}): ${url}`, errorMsg); + } + throw new Error(`Git Service Error (${response.status}): ${errorMsg}`); + } + + return response; } protected abstract addAuthHeader(headers: Record): void; diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 2ccd5eb..5a91755 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -3,4 +3,5 @@ const PREFIX = '[git-file-sync]'; export const logger = { error: (message: string, ...args: unknown[]) => console.error(`${PREFIX} ${message}`, ...args), warn: (message: string, ...args: unknown[]) => console.warn(`${PREFIX} ${message}`, ...args), + debug: (message: string, ...args: unknown[]) => console.debug(`${PREFIX} ${message}`, ...args), }; diff --git a/tests/services/git-service-base.test.ts b/tests/services/git-service-base.test.ts index ecb5057..e5eb8ba 100644 --- a/tests/services/git-service-base.test.ts +++ b/tests/services/git-service-base.test.ts @@ -30,6 +30,42 @@ describe('BaseGitService', () => { }); }); + describe('safeRequest 404 handling', () => { + it('getFile returns empty and does not log an error on 404 (e.g. missing .gitignore)', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); + vi.mocked(requestUrl).mockResolvedValue({ + status: 404, + text: 'Not Found', + json: { message: 'Not Found' }, + } as unknown as RequestUrlResponse); + + const result = await service.getFile('missing/.gitignore', 'main'); + + expect(result).toEqual({ content: '', sha: '' }); + // A 404 is an expected "does not exist" probe: never logged as an error… + expect(errorSpy).not.toHaveBeenCalled(); + // …and logged at most once at debug level (no double-logging). + expect(debugSpy).toHaveBeenCalledTimes(1); + + errorSpy.mockRestore(); + debugSpy.mockRestore(); + }); + + it('still logs non-404 failures as errors', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.mocked(requestUrl).mockResolvedValue({ + status: 500, + text: 'Internal Server Error', + json: { message: 'Internal Server Error' }, + } as unknown as RequestUrlResponse); + + await expect(service.listFiles('main')).rejects.toThrow('500'); + expect(errorSpy).toHaveBeenCalledTimes(1); + errorSpy.mockRestore(); + }); + }); + describe('safeRequest with non-Error exception', () => { it('should wrap non-Error throws in a new Error', async () => { // Throw a plain string (not an Error instance) from requestUrl