Update send function to propagate any errors and fix catchErrors to return the error in result when an unknown command/method is sent to the browser server (#6942)

* Fix send not returning error when catchErrors option is enabled and an unknown method error is encountered

* Add release notes for PR #6942

* Fix send to properly propagate errors from the server

* Update release note

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

packages/loot-core/src/platform/client/fetch/__mocks__/index.ts

@@ -14,7 +14,7 @@ export const initServer: T.InitServer = handlers => {
         if (catchErrors) {
           return promise.then(
             data => ({ data }),
-            err => ({ error: { message: err.message } }),
+            error => ({ error }),
           );
         }
         return promise;

packages/loot-core/src/platform/client/fetch/index-types.ts

@@ -4,6 +4,17 @@ import type { ServerEvents } from '../../../types/server-events';
 export declare function init(worker: Worker): Promise<unknown>;
 export type Init = typeof init;
 
+/**
+ * Send a command to the browser server.
+ *
+ * @param name The name of the command to be executed by the browser server.
+ * @param args The command arguments.
+ * @param options The options for the command. If `catchErrors` is true,
+ * and an error occurs, the promise will be resolved with an object that
+ * has an `error` property. Otherwise, the promise will be rejected with the error.
+ * @returns A promise that resolves with the command result, or rejects with an error if one occurs.
+ * If you want to catch errors as part of the resolved value instead of rejecting, use `sendCatch` instead.
+ */
 export declare function send<K extends keyof Handlers>(
   name: K,
   args: Parameters<Handlers[K]>[0],
@@ -22,6 +33,14 @@ export declare function send<K extends keyof Handlers>(
 ): Promise<Awaited<ReturnType<Handlers[K]>>>;
 export type Send = typeof send;
 
+/**
+ * Send a command to the browser server.
+ *
+ * @param name The name of the command to be executed by the browser server.
+ * @param args The command arguments.
+ * @returns A promise that resolves with an object containing either the command result or an error if one occurs.
+ * The promise will never reject, as errors are caught and returned as part of the resolved value.
+ */
 export declare function sendCatch<K extends keyof Handlers>(
   name: K,
   args?: Parameters<Handlers[K]>[0],
@@ -34,12 +53,26 @@ export declare function sendCatch<K extends keyof Handlers>(
 >;
 export type SendCatch = typeof sendCatch;
 
+/** Server push listeners */
+
+/**
+ * Listen to events pushed to the client from the browser server.
+ *
+ * @param name The name of the event to listen to.
+ * @param cb The callback to be called when the event is received.
+ * @returns A function that can be called to unregister the listener.
+ */
 export declare function listen<K extends keyof ServerEvents>(
   name: K,
   cb: (arg: ServerEvents[K]) => void,
 ): () => void;
 export type Listen = typeof listen;
 
+/**
+ * Stop listening to events pushed to the client from the browser server.
+ *
+ * @param name The name of the event to stop listening to.
+ */
 export declare function unlisten(name: string): void;
 export type Unlisten = typeof unlisten;
 

packages/loot-core/src/platform/client/fetch/index.browser.ts

@@ -42,12 +42,14 @@ class ReconstructedError extends Error {
 function handleMessage(msg) {
   if (msg.type === 'error') {
     // An error happened while handling a message so cleanup the
-    // current reply handler. We don't care about the actual error -
-    // generic backend errors are handled separately and if you want
-    // more specific handling you should manually forward the error
-    // through a normal reply.
-    const { id } = msg;
-    replyHandlers.delete(id);
+    // current reply handler and reject the promise. The error will
+    // be propagated to the caller through this promise rejection.
+    const { id, error } = msg;
+    const handler = replyHandlers.get(id);
+    if (handler) {
+      replyHandlers.delete(id);
+      handler.reject(error);
+    }
   } else if (msg.type === 'reply') {
     const { id, result, mutated, undoTag } = msg;
 

packages/loot-core/src/platform/client/fetch/index.ts

@@ -17,12 +17,14 @@ function connectSocket(onOpen) {
 
       if (msg.type === 'error') {
         // An error happened while handling a message so cleanup the
-        // current reply handler. We don't care about the actual error -
-        // generic backend errors are handled separately and if you want
-        // more specific handling you should manually forward the error
-        // through a normal reply.
-        const { id } = msg;
-        replyHandlers.delete(id);
+        // current reply handler and reject the promise. The error will
+        // be propagated to the caller through this promise rejection.
+        const { id, error } = msg;
+        const handler = replyHandlers.get(id);
+        if (handler) {
+          replyHandlers.delete(id);
+          handler.reject(error);
+        }
       } else if (msg.type === 'reply') {
         let { result } = msg;
         const { id, mutated, undoTag } = msg;

packages/loot-core/src/platform/server/connection/index.electron.ts

@@ -52,7 +52,7 @@ export const init: T.Init = function (_socketName, handlers) {
               result: { error, data: null },
             });
           } else {
-            process.parentPort.postMessage({ type: 'error', id });
+            process.parentPort.postMessage({ type: 'error', id, error });
           }
 
           if (error.type === 'InternalError' && name !== 'api/load-budget') {
@@ -66,14 +66,25 @@ export const init: T.Init = function (_socketName, handlers) {
         },
       );
     } else {
-      logger.warn('Unknown method: ' + name);
+      logger.error('Unknown server method: ' + name);
       captureException(new Error('Unknown server method: ' + name));
-      process.parentPort.postMessage({
-        type: 'reply',
-        id,
-        result: null,
-        error: APIError('Unknown method: ' + name),
-      });
+      const unknownMethodError = APIError('Unknown server method: ' + name);
+
+      if (catchErrors) {
+        process.parentPort.postMessage({
+          type: 'reply',
+          id,
+          result: catchErrors
+            ? { error: unknownMethodError, data: null }
+            : null,
+        });
+      } else {
+        process.parentPort.postMessage({
+          type: 'error',
+          id,
+          error: unknownMethodError,
+        });
+      }
     }
   });
 };

packages/loot-core/src/platform/server/connection/index.ts

@@ -79,7 +79,7 @@ export const init: T.Init = function (serverChn, handlers) {
                 result: { error, data: null },
               });
             } else {
-              serverChannel.postMessage({ type: 'error', id });
+              serverChannel.postMessage({ type: 'error', id, error });
             }
 
             // Only report internal errors
@@ -94,13 +94,25 @@ export const init: T.Init = function (serverChn, handlers) {
           },
         );
       } else {
-        logger.warn('Unknown method: ' + name);
-        serverChannel.postMessage({
-          type: 'reply',
-          id,
-          result: null,
-          error: APIError('Unknown method: ' + name),
-        });
+        logger.error('Unknown server method: ' + name);
+        captureException(new Error('Unknown server method: ' + name));
+        const unknownMethodError = APIError('Unknown server method: ' + name);
+
+        if (catchErrors) {
+          serverChannel.postMessage({
+            type: 'reply',
+            id,
+            result: catchErrors
+              ? { error: unknownMethodError, data: null }
+              : null,
+          });
+        } else {
+          serverChannel.postMessage({
+            type: 'error',
+            id,
+            error: unknownMethodError,
+          });
+        }
       }
     },
     false,