fix(errors): Properly assign properties on hardened ocap errors

packages/errors/src/BaseError.test.ts

@@ -9,6 +9,7 @@ describe('BaseError', () => {
   const mockMessage = 'VAT was not found.';
   const mockData = { key: 'value' };
   const mockCause = new Error('Root cause error');
+  const mockStack = 'Error stack';
 
   it('creates a BaseError with required properties', () => {
     const error = new BaseError(mockCode, mockMessage);
@@ -22,12 +23,17 @@ describe('BaseError', () => {
   });
 
   it('creates a BaseError with all properties', () => {
-    const error = new BaseError(mockCode, mockMessage, mockData, mockCause);
+    const error = new BaseError(mockCode, mockMessage, {
+      data: mockData,
+      cause: mockCause,
+      stack: mockStack,
+    });
     expect(error.name).toBe('BaseError');
     expect(error.message).toBe(mockMessage);
     expect(error.code).toBe(mockCode);
     expect(error.data).toStrictEqual(mockData);
     expect(error.cause).toBe(mockCause);
+    expect(error.stack).toBe(mockStack);
   });
 
   it('inherits from the Error class and have the correct name', () => {
@@ -37,13 +43,15 @@ describe('BaseError', () => {
   });
 
   it('handles a missing data parameter', () => {
-    const error = new BaseError(mockCode, mockMessage, undefined, mockCause);
+    const error = new BaseError(mockCode, mockMessage, {
+      cause: mockCause,
+    });
     expect(error.data).toBeUndefined();
     expect(error.cause).toBe(mockCause);
   });
 
   it('handles a missing cause parameter', () => {
-    const error = new BaseError(mockCode, mockMessage, mockData);
+    const error = new BaseError(mockCode, mockMessage, { data: mockData });
     expect(error.data).toStrictEqual(mockData);
     expect(error.cause).toBeUndefined();
   });
@@ -53,4 +61,16 @@ describe('BaseError', () => {
       'Unmarshal method not implemented',
     );
   });
+
+  it('initializes the stack property automatically if not provided', () => {
+    const error = new BaseError(mockCode, mockMessage, { cause: mockCause });
+    expect(error.stack).toBeDefined();
+  });
+
+  it('creates a BaseError with a custom stack', () => {
+    const error = new BaseError(mockCode, mockMessage, { stack: mockStack });
+    expect(error.stack).toBe(mockStack);
+    expect(error.data).toBeUndefined();
+    expect(error.cause).toBeUndefined();
+  });
 });

packages/errors/src/BaseError.ts

@@ -1,20 +1,37 @@
 import type { Json } from '@metamask/utils';
 
 import type { ErrorCode } from './constants.js';
-import type { MarshaledOcapError, OcapError } from './types.js';
+import type {
+  MarshaledOcapError,
+  OcapError,
+  ErrorOptionsWithStack,
+} from './types.js';
 
 export class BaseError extends Error implements OcapError {
   public readonly code: ErrorCode;
 
   public readonly data: Json | undefined;
 
-  constructor(code: ErrorCode, message: string, data?: Json, cause?: unknown) {
+  constructor(
+    code: ErrorCode,
+    message: string,
+    options: ErrorOptionsWithStack & {
+      data?: Json;
+    } = {},
+  ) {
+    const { data, cause, stack } = options;
+
     super(message, { cause });
 
     this.name = this.constructor.name;
     this.code = code;
     this.data = data;
-    this.cause = cause;
+
+    // override the stack property if provided
+    if (stack) {
+      this.stack = stack;
+    }
+
     harden(this);
   }
 

packages/errors/src/errors/StreamReadError.test.ts

@@ -12,7 +12,7 @@ describe('StreamReadError', () => {
   it('creates a StreamReadError for Supervisor with the correct properties', () => {
     const error = new StreamReadError(
       { supervisorId: mockSupervisorId },
-      mockOriginalError,
+      { cause: mockOriginalError },
     );
     expect(error).toBeInstanceOf(StreamReadError);
     expect(error.code).toBe(ErrorCode.StreamReadError);
@@ -22,7 +22,10 @@ describe('StreamReadError', () => {
   });
 
   it('creates a StreamReadError for Vat with the correct properties', () => {
-    const error = new StreamReadError({ vatId: mockVatId }, mockOriginalError);
+    const error = new StreamReadError(
+      { vatId: mockVatId },
+      { cause: mockOriginalError },
+    );
     expect(error).toBeInstanceOf(StreamReadError);
     expect(error.code).toBe(ErrorCode.StreamReadError);
     expect(error.message).toBe('Unexpected stream read error.');
@@ -49,6 +52,7 @@ describe('StreamReadError', () => {
     expect(unmarshaledError).toBeInstanceOf(StreamReadError);
     expect(unmarshaledError.code).toBe(ErrorCode.StreamReadError);
     expect(unmarshaledError.message).toBe('Unexpected stream read error.');
+    expect(unmarshaledError.stack).toBe('customStack');
     expect(unmarshaledError.data).toStrictEqual({
       vatId: mockVatId,
     });
@@ -75,6 +79,7 @@ describe('StreamReadError', () => {
     expect(unmarshaledError).toBeInstanceOf(StreamReadError);
     expect(unmarshaledError.code).toBe(ErrorCode.StreamReadError);
     expect(unmarshaledError.message).toBe('Unexpected stream read error.');
+    expect(unmarshaledError.stack).toBe('customStack');
     expect(unmarshaledError.data).toStrictEqual({
       supervisorId: mockSupervisorId,
     });

packages/errors/src/errors/StreamReadError.ts

@@ -14,18 +14,19 @@ import {
   ErrorCode,
   MarshaledErrorStruct,
 } from '../constants.js';
-import type { MarshaledOcapError } from '../types.js';
+import { unmarshalErrorOptions } from '../marshal/unmarshalError.js';
+import type { ErrorOptionsWithStack, MarshaledOcapError } from '../types.js';
 
 type StreamReadErrorData = { vatId: string } | { supervisorId: string };
+type StreamReadErrorOptions = Required<ErrorOptions> &
+  Pick<ErrorOptionsWithStack, 'stack'>;
 
 export class StreamReadError extends BaseError {
-  constructor(data: StreamReadErrorData, originalError: Error) {
-    super(
-      ErrorCode.StreamReadError,
-      'Unexpected stream read error.',
+  constructor(data: StreamReadErrorData, options: StreamReadErrorOptions) {
+    super(ErrorCode.StreamReadError, 'Unexpected stream read error.', {
+      ...options,
       data,
-      originalError,
-    );
+    });
     harden(this);
   }
 
@@ -52,8 +53,7 @@ export class StreamReadError extends BaseError {
     assert(marshaledError, this.struct);
     return new StreamReadError(
       marshaledError.data as StreamReadErrorData,
-      // The cause will be properly unmarshaled during the parent call.
-      new Error(marshaledError.cause?.message),
+      unmarshalErrorOptions(marshaledError) as StreamReadErrorOptions,
     );
   }
 }

packages/errors/src/errors/VatAlreadyExistsError.test.ts

@@ -29,6 +29,7 @@ describe('VatAlreadyExistsError', () => {
     expect(unmarshaledError).toBeInstanceOf(VatAlreadyExistsError);
     expect(unmarshaledError.code).toBe(ErrorCode.VatAlreadyExists);
     expect(unmarshaledError.message).toBe('Vat already exists.');
+    expect(unmarshaledError.stack).toBe('stack trace');
     expect(unmarshaledError.data).toStrictEqual({
       vatId: mockVatId,
     });

packages/errors/src/errors/VatAlreadyExistsError.ts

@@ -2,12 +2,14 @@ import { assert, literal, object, string } from '@metamask/superstruct';
 
 import { BaseError } from '../BaseError.js';
 import { marshaledErrorSchema, ErrorCode } from '../constants.js';
-import type { MarshaledOcapError } from '../types.js';
+import { unmarshalErrorOptions } from '../marshal/unmarshalError.js';
+import type { ErrorOptionsWithStack, MarshaledOcapError } from '../types.js';
 
 export class VatAlreadyExistsError extends BaseError {
-  constructor(vatId: string) {
+  constructor(vatId: string, options?: ErrorOptionsWithStack) {
     super(ErrorCode.VatAlreadyExists, 'Vat already exists.', {
-      vatId,
+      ...options,
+      data: { vatId },
     });
     harden(this);
   }
@@ -33,7 +35,10 @@ export class VatAlreadyExistsError extends BaseError {
     marshaledError: MarshaledOcapError,
   ): VatAlreadyExistsError {
     assert(marshaledError, this.struct);
-    return new VatAlreadyExistsError(marshaledError.data.vatId);
+    return new VatAlreadyExistsError(
+      marshaledError.data.vatId,
+      unmarshalErrorOptions(marshaledError),
+    );
   }
 }
 harden(VatAlreadyExistsError);

packages/errors/src/errors/VatCapTpConnectionExistsError.test.ts

@@ -29,6 +29,7 @@ describe('VatCapTpConnectionExistsError', () => {
       VatCapTpConnectionExistsError.unmarshal(marshaledError);
     expect(unmarshaledError).toBeInstanceOf(VatCapTpConnectionExistsError);
     expect(unmarshaledError.code).toBe(ErrorCode.VatCapTpConnectionExists);
+    expect(unmarshaledError.stack).toBe('stack trace');
     expect(unmarshaledError.message).toBe(
       'Vat already has a CapTP connection.',
     );

packages/errors/src/errors/VatCapTpConnectionExistsError.ts

@@ -2,15 +2,17 @@ import { assert, literal, object, string } from '@metamask/superstruct';
 
 import { BaseError } from '../BaseError.js';
 import { marshaledErrorSchema, ErrorCode } from '../constants.js';
-import type { MarshaledOcapError } from '../types.js';
+import { unmarshalErrorOptions } from '../marshal/unmarshalError.js';
+import type { ErrorOptionsWithStack, MarshaledOcapError } from '../types.js';
 
 export class VatCapTpConnectionExistsError extends BaseError {
-  constructor(vatId: string) {
+  constructor(vatId: string, options?: ErrorOptionsWithStack) {
     super(
       ErrorCode.VatCapTpConnectionExists,
       'Vat already has a CapTP connection.',
       {
-        vatId,
+        ...options,
+        data: { vatId },
       },
     );
     harden(this);
@@ -37,7 +39,10 @@ export class VatCapTpConnectionExistsError extends BaseError {
     marshaledError: MarshaledOcapError,
   ): VatCapTpConnectionExistsError {
     assert(marshaledError, this.struct);
-    return new VatCapTpConnectionExistsError(marshaledError.data.vatId);
+    return new VatCapTpConnectionExistsError(
+      marshaledError.data.vatId,
+      unmarshalErrorOptions(marshaledError),
+    );
   }
 }
 harden(VatCapTpConnectionExistsError);

packages/errors/src/errors/VatCapTpConnectionNotFoundError.test.ts

@@ -29,6 +29,7 @@ describe('VatCapTpConnectionNotFoundError', () => {
       VatCapTpConnectionNotFoundError.unmarshal(marshaledError);
     expect(unmarshaledError).toBeInstanceOf(VatCapTpConnectionNotFoundError);
     expect(unmarshaledError.code).toBe(ErrorCode.VatCapTpConnectionNotFound);
+    expect(unmarshaledError.stack).toBe('stack trace');
     expect(unmarshaledError.message).toBe(
       'Vat does not have a CapTP connection.',
     );

packages/errors/src/errors/VatCapTpConnectionNotFoundError.ts

@@ -2,14 +2,18 @@ import { assert, literal, object, string } from '@metamask/superstruct';
 
 import { BaseError } from '../BaseError.js';
 import { marshaledErrorSchema, ErrorCode } from '../constants.js';
-import type { MarshaledOcapError } from '../types.js';
+import { unmarshalErrorOptions } from '../marshal/unmarshalError.js';
+import type { ErrorOptionsWithStack, MarshaledOcapError } from '../types.js';
 
 export class VatCapTpConnectionNotFoundError extends BaseError {
-  constructor(vatId: string) {
+  constructor(vatId: string, options?: ErrorOptionsWithStack) {
     super(
       ErrorCode.VatCapTpConnectionNotFound,
       'Vat does not have a CapTP connection.',
-      { vatId },
+      {
+        ...options,
+        data: { vatId },
+      },
     );
     harden(this);
   }
@@ -35,7 +39,10 @@ export class VatCapTpConnectionNotFoundError extends BaseError {
     marshaledError: MarshaledOcapError,
   ): VatCapTpConnectionNotFoundError {
     assert(marshaledError, this.struct);
-    return new VatCapTpConnectionNotFoundError(marshaledError.data.vatId);
+    return new VatCapTpConnectionNotFoundError(
+      marshaledError.data.vatId,
+      unmarshalErrorOptions(marshaledError),
+    );
   }
 }
 harden(VatCapTpConnectionNotFoundError);

packages/errors/src/errors/VatDeletedError.test.ts

@@ -29,6 +29,7 @@ describe('VatDeletedError', () => {
     expect(unmarshaledError).toBeInstanceOf(VatDeletedError);
     expect(unmarshaledError.code).toBe(ErrorCode.VatDeleted);
     expect(unmarshaledError.message).toBe('Vat was deleted.');
+    expect(unmarshaledError.stack).toBe('stack trace');
     expect(unmarshaledError.data).toStrictEqual({
       vatId: mockVatId,
     });

packages/errors/src/errors/VatDeletedError.ts

@@ -2,11 +2,15 @@ import { assert, literal, object, string } from '@metamask/superstruct';
 
 import { BaseError } from '../BaseError.js';
 import { marshaledErrorSchema, ErrorCode } from '../constants.js';
-import type { MarshaledOcapError } from '../types.js';
+import { unmarshalErrorOptions } from '../marshal/unmarshalError.js';
+import type { ErrorOptionsWithStack, MarshaledOcapError } from '../types.js';
 
 export class VatDeletedError extends BaseError {
-  constructor(vatId: string) {
-    super(ErrorCode.VatDeleted, 'Vat was deleted.', { vatId });
+  constructor(vatId: string, options?: ErrorOptionsWithStack) {
+    super(ErrorCode.VatDeleted, 'Vat was deleted.', {
+      ...options,
+      data: { vatId },
+    });
     harden(this);
   }
 
@@ -29,7 +33,10 @@ export class VatDeletedError extends BaseError {
    */
   public static unmarshal(marshaledError: MarshaledOcapError): VatDeletedError {
     assert(marshaledError, this.struct);
-    return new VatDeletedError(marshaledError.data.vatId);
+    return new VatDeletedError(
+      marshaledError.data.vatId,
+      unmarshalErrorOptions(marshaledError),
+    );
   }
 }
 harden(VatDeletedError);

packages/errors/src/errors/VatNotFoundError.test.ts

@@ -29,6 +29,7 @@ describe('VatNotFoundError', () => {
     expect(unmarshaledError).toBeInstanceOf(VatNotFoundError);
     expect(unmarshaledError.code).toBe(ErrorCode.VatNotFound);
     expect(unmarshaledError.message).toBe('Vat does not exist.');
+    expect(unmarshaledError.stack).toBe('stack trace');
     expect(unmarshaledError.data).toStrictEqual({
       vatId: mockVatId,
     });

packages/errors/src/errors/VatNotFoundError.ts

@@ -2,11 +2,15 @@ import { assert, literal, object, string } from '@metamask/superstruct';
 
 import { BaseError } from '../BaseError.js';
 import { marshaledErrorSchema, ErrorCode } from '../constants.js';
-import type { MarshaledOcapError } from '../types.js';
+import { unmarshalErrorOptions } from '../marshal/unmarshalError.js';
+import type { ErrorOptionsWithStack, MarshaledOcapError } from '../types.js';
 
 export class VatNotFoundError extends BaseError {
-  constructor(vatId: string) {
-    super(ErrorCode.VatNotFound, 'Vat does not exist.', { vatId });
+  constructor(vatId: string, options?: ErrorOptionsWithStack) {
+    super(ErrorCode.VatNotFound, 'Vat does not exist.', {
+      ...options,
+      data: { vatId },
+    });
     harden(this);
   }
 
@@ -31,7 +35,10 @@ export class VatNotFoundError extends BaseError {
     marshaledError: MarshaledOcapError,
   ): VatNotFoundError {
     assert(marshaledError, this.struct);
-    return new VatNotFoundError(marshaledError.data.vatId);
+    return new VatNotFoundError(
+      marshaledError.data.vatId,
+      unmarshalErrorOptions(marshaledError),
+    );
   }
 }
 harden(VatNotFoundError);

packages/errors/src/marshal/marshal.test.ts

@@ -0,0 +1,14 @@
+import { describe, it, expect } from 'vitest';
+
+import { marshalError } from './marshalError.js';
+import { unmarshalError } from './unmarshalError.js';
+import { VatAlreadyExistsError } from '../errors/VatAlreadyExistsError.js';
+
+describe('marshal', () => {
+  it('should round trip a thrown error', async () => {
+    const thrown = new VatAlreadyExistsError('v123');
+    const marshaled = marshalError(thrown);
+    const received = unmarshalError(marshaled);
+    expect(received).toStrictEqual(thrown);
+  });
+});