Desktop, Mobile, Cli: Fixes #16504: Fix sync deadlock when Joplin Server rejects an item with HTTP 422 (#16544)

.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

.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

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();
+	});
+
+});

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 }) {