From 330b86e63483fe3c8da377d8ae185422914ad69f Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 13 Sep 2026 11:37:43 +0100 Subject: [PATCH] Desktop, Mobile, Cli: Fixes #16504: Fix sync deadlock when Joplin Server rejects an item with HTTP 422 (#16544) --- .gitignore | 1 + .ignore.eslint | 1 + .../lib/file-api-driver-joplinServer.test.ts | 62 +++++++++++++++++++ packages/lib/file-api-driver-joplinServer.ts | 6 +- 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 packages/lib/file-api-driver-joplinServer.test.ts diff --git a/.gitignore b/.gitignore index bc904fbf70..2cfa50e175 100644 --- a/.gitignore +++ b/.gitignore @@ -1442,6 +1442,7 @@ packages/lib/errorUtils.js packages/lib/errors.js packages/lib/eventManager.test.js packages/lib/eventManager.js +packages/lib/file-api-driver-joplinServer.test.js packages/lib/file-api-driver-joplinServer.js packages/lib/file-api-driver-local.js packages/lib/file-api-driver-memory.js diff --git a/.ignore.eslint b/.ignore.eslint index bb3fbc7115..2d5bcefa6d 100644 --- a/.ignore.eslint +++ b/.ignore.eslint @@ -1468,6 +1468,7 @@ packages/lib/errorUtils.js packages/lib/errors.js packages/lib/eventManager.test.js packages/lib/eventManager.js +packages/lib/file-api-driver-joplinServer.test.js packages/lib/file-api-driver-joplinServer.js packages/lib/file-api-driver-local.js packages/lib/file-api-driver-memory.js diff --git a/packages/lib/file-api-driver-joplinServer.test.ts b/packages/lib/file-api-driver-joplinServer.test.ts new file mode 100644 index 0000000000..74320c0e2f --- /dev/null +++ b/packages/lib/file-api-driver-joplinServer.test.ts @@ -0,0 +1,62 @@ +import FileApiDriverJoplinServer from './file-api-driver-joplinServer'; +import JoplinServerApi from './JoplinServerApi'; + +// Errors as they are returned by JoplinServerApi when the server rejects a +// request - see JoplinServerApi.exec() +const serverError = (httpCode: number, message: string) => { + const error = new Error(message) as Error & { code: number; httpCode: number }; + error.code = httpCode; + error.httpCode = httpCode; + return error; +}; + +const newDriver = (execHandler: ()=> unknown) => { + const api = { exec: execHandler } as unknown as JoplinServerApi; + return new FileApiDriverJoplinServer(api); +}; + +describe('file-api-driver-joplinServer', () => { + + test.each([ + [409, 'Conflict'], + [413, 'Payload too large'], + [422, 'Item 1.md cannot be saved because its body contains a null byte'], + ])('should convert a %s error on put() to a rejectedByTarget error', async (httpCode, message) => { + const driver = newDriver(() => { + throw serverError(httpCode, message); + }); + + await expect(driver.put('1.md', 'content')).rejects.toMatchObject({ + code: 'rejectedByTarget', + message, + }); + }); + + test('should not convert unrelated errors on put()', async () => { + const driver = newDriver(() => { + throw serverError(500, 'Internal server error'); + }); + + // Such errors must keep bubbling up so that the sync is retried later + await expect(driver.put('1.md', 'content')).rejects.toMatchObject({ + code: 500, + }); + }); + + test('should flag items rejected by the target in a multiPut() response', async () => { + const driver = newDriver(() => ({ + items: { + '1.md': { error: serverError(422, 'body contains a null byte') }, + '2.md': { error: serverError(500, 'Internal server error') }, + '3.md': {}, + }, + })); + + const output = await driver.multiPut([]); + + expect(output.items['1.md'].error.code).toBe('rejectedByTarget'); + expect(output.items['2.md'].error.code).toBe(500); + expect(output.items['3.md'].error).toBeUndefined(); + }); + +}); diff --git a/packages/lib/file-api-driver-joplinServer.ts b/packages/lib/file-api-driver-joplinServer.ts index de6878efcb..14b16c074f 100644 --- a/packages/lib/file-api-driver-joplinServer.ts +++ b/packages/lib/file-api-driver-joplinServer.ts @@ -186,7 +186,11 @@ export default class FileApiDriverJoplinServer { } private isRejectedBySyncTargetError(error: { code?: number | string; httpCode?: number }) { - return error.code === 413 || error.code === 409 || error.httpCode === 413 || error.httpCode === 409; + // 422 means the server permanently refuses the item (for example its + // content contains a null byte). Without it the sync would abort and + // retry the same item forever. + const rejectedCodes = [409, 413, 422]; + return rejectedCodes.includes(error.code as number) || rejectedCodes.includes(error.httpCode); } private isReadyOnlyError(error: { code?: string }) {