fix(transport): propagate error message instead of returning unexpect…

…ed error
trezor · Nov 25, 2024 · 580a6d2 · 580a6d2
1 parent cba164c
commit 580a6d2
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 32 deletions.
diff --git a/packages/connect/src/device/DeviceCommands.ts b/packages/connect/src/device/DeviceCommands.ts
@@ -320,7 +320,12 @@ export class DeviceCommands {
 
         this.callPromise = undefined;
         if (!res.success) {
-            logger.warn('Received error', res.error);
+            logger.warn(
+                'Received error',
+                res.error,
+                // res.message is not propagated to higher levels, only logged here. webusb/node-bridge may return message with additional information
+                res.message,
+            );
             throw new Error(res.error);
         }
 

diff --git a/packages/transport-bridge/src/http.ts b/packages/transport-bridge/src/http.ts
@@ -16,6 +16,7 @@ import { Descriptor, PathPublic, Session } from '@trezor/transport/src/types';
 import { validateProtocolMessage } from '@trezor/transport/src/utils/bridgeProtocolMessage';
 import { Log, arrayPartition, Throttler } from '@trezor/utils';
 import { AbstractApi } from '@trezor/transport/src/api/abstract';
+import { UNEXPECTED_ERROR } from '@trezor/transport/src/errors';
 
 import { createCore } from './core';
 
@@ -78,7 +79,7 @@ const validateProtocolMessageBody =
             return next({ ...request, body }, response);
         } catch (error) {
             response.statusCode = 400;
-            response.end(str({ error: error.message }));
+            response.end(str({ error: UNEXPECTED_ERROR, message: error.message }));
         }
     };
 
@@ -250,7 +251,7 @@ export class TrezordNode {
                         if (!result.success) {
                             res.statusCode = 400;
 
-                            return res.end(str({ error: result.error }));
+                            return res.end(str({ error: result.error, message: result.message }));
                         }
                         res.end(str(result.payload.descriptors));
                     });
@@ -289,7 +290,9 @@ export class TrezordNode {
                             if (!result.success) {
                                 res.statusCode = 400;
 
-                                return res.end(str({ error: result.error }));
+                                return res.end(
+                                    str({ error: result.error, message: result.message }),
+                                );
                             }
                             res.end(str({ session: result.payload.session }));
                         });
@@ -308,7 +311,9 @@ export class TrezordNode {
                             if (!result.success) {
                                 res.statusCode = 400;
 
-                                return res.end(str({ error: result.error }));
+                                return res.end(
+                                    str({ error: result.error, message: result.message }),
+                                );
                             }
                             res.end(str({ session: req.params.session }));
                         });
@@ -331,7 +336,9 @@ export class TrezordNode {
                             if (!result.success) {
                                 res.statusCode = 400;
 
-                                return res.end(str({ error: result.error }));
+                                return res.end(
+                                    str({ error: result.error, message: result.message }),
+                                );
                             }
                             res.end(str(result.payload));
                         });
@@ -354,7 +361,9 @@ export class TrezordNode {
                             if (!result.success) {
                                 res.statusCode = 400;
 
-                                return res.end(str({ error: result.error }));
+                                return res.end(
+                                    str({ error: result.error, message: result.message }),
+                                );
                             }
                             res.end(str(result.payload));
                         });
@@ -377,7 +386,9 @@ export class TrezordNode {
                             if (!result.success) {
                                 res.statusCode = 400;
 
-                                return res.end(str({ error: result.error }));
+                                return res.end(
+                                    str({ error: result.error, message: result.message }),
+                                );
                             }
                             res.end(str(result.payload));
                         });

diff --git a/packages/transport-bridge/tests/http.test.ts b/packages/transport-bridge/tests/http.test.ts
@@ -320,6 +320,7 @@ describe('http', () => {
             });
             expect(res).toMatchObject({
                 success: false,
+                error: 'unexpected error',
                 message: 'Invalid BridgeProtocolMessage protocol',
             });
 

diff --git a/packages/transport/src/api/usb.ts b/packages/transport/src/api/usb.ts
@@ -219,10 +219,7 @@ export class UsbApi extends AbstractApi {
                 return this.error({ error: ERRORS.DEVICE_DISCONNECTED_DURING_ACTION });
             }
 
-            return this.unknownError(err, [
-                ERRORS.ABORTED_BY_SIGNAL,
-                ERRORS.INTERFACE_DATA_TRANSFER,
-            ]);
+            return this.error({ error: ERRORS.INTERFACE_DATA_TRANSFER, message: err.message });
         }
     }
 

diff --git a/packages/transport/src/transports/bridge.ts b/packages/transport/src/transports/bridge.ts
@@ -364,7 +364,7 @@ export class BridgeTransport extends AbstractTransport {
 
         if (!response.success) {
             if (response.error === ERRORS.UNEXPECTED_ERROR) {
-                return response;
+                return this.unknownError(response.error);
             }
             if (response.error === ERRORS.HTTP_ERROR) {
                 return this.error({ error: response.error });
@@ -380,24 +380,19 @@ export class BridgeTransport extends AbstractTransport {
                         ERRORS.INTERFACE_UNABLE_TO_OPEN_DEVICE,
                         ERRORS.DEVICE_DISCONNECTED_DURING_ACTION,
                     ]);
-                case '/read':
                 case '/call':
+                case '/read':
+                case '/post':
                     return this.unknownError(response.error, [
                         ERRORS.SESSION_NOT_FOUND,
                         ERRORS.DEVICE_DISCONNECTED_DURING_ACTION,
                         ERRORS.OTHER_CALL_IN_PROGRESS,
+                        ERRORS.INTERFACE_DATA_TRANSFER,
                         PROTOCOL_MALFORMED,
                     ]);
                 case '/enumerate':
                 case '/listen':
                     return this.unknownError(response.error);
-                case '/post':
-                    return this.unknownError(response.error, [
-                        ERRORS.SESSION_NOT_FOUND,
-                        ERRORS.DEVICE_DISCONNECTED_DURING_ACTION,
-                        ERRORS.OTHER_CALL_IN_PROGRESS,
-                        PROTOCOL_MALFORMED,
-                    ]);
                 case '/release':
                     return this.unknownError(response.error, [
                         ERRORS.SESSION_NOT_FOUND,

diff --git a/packages/transport/src/types/apiCall.ts b/packages/transport/src/types/apiCall.ts
@@ -9,16 +9,11 @@ export interface Success<T> {
     payload: T;
 }
 
-export type ErrorGeneric<ErrorType> = ErrorType extends AnyError
-    ? {
-          success: false;
-          error: ErrorType;
-      }
-    : {
-          success: false;
-          error: ErrorType;
-          message?: string;
-      };
+export type ErrorGeneric<ErrorType> = {
+    success: false;
+    error: ErrorType;
+    message?: string;
+};
 
 export type ResultWithTypedError<T, E> = Success<T> | ErrorGeneric<E>;
 export type AsyncResultWithTypedError<T, E> = Promise<Success<T> | ErrorGeneric<E>>;

diff --git a/packages/transport/src/utils/bridgeApiCall.ts b/packages/transport/src/utils/bridgeApiCall.ts
@@ -82,6 +82,7 @@ export async function bridgeApiCall(options: HttpRequestOptions) {
 
     // if status is not 200. response should be interpreted as error.
     if (!res.ok) {
+        // this block only changes error messages from old bridge to the same messages returned by new bridge / connect usb stack
         const errStr =
             typeof resParsed !== 'string' && 'error' in resParsed
                 ? (resParsed.error as string)
@@ -97,6 +98,20 @@ export async function bridgeApiCall(options: HttpRequestOptions) {
             return error({ error: PROTOCOL_MALFORMED });
         }
 
+        if (
+            typeof resParsed !== 'string' &&
+            'error' in resParsed &&
+            typeof resParsed.error === 'string'
+        ) {
+            return error({
+                error: resParsed.error,
+                message:
+                    'message' in resParsed && typeof resParsed.message === 'string'
+                        ? resParsed.message
+                        : undefined,
+            });
+        }
+
         return unknownError(new Error(errStr), Object.values(ERRORS));
     }
 

diff --git a/packages/transport/tests/apiUsb.test.ts b/packages/transport/tests/apiUsb.test.ts
@@ -58,7 +58,7 @@ describe('api/usb', () => {
 
         const result = await promise;
         if (result.success) throw new Error('Unexpected success');
-        expect(result.error).toContain('Aborted by signal');
+        expect(result.error).toContain('A transfer error has occurred.');
         expect(reset).toHaveBeenCalledTimes(1);
     });