Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/ocap-kernel/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Attribute a failed subcluster vat launch to the specific vat by kernel id and `ClusterConfig` name (e.g. `Failed to launch vat v3 (bob)`), preserving the original error as the `cause` ([#975](https://github.com/MetaMask/ocap-kernel/pull/975))
- **BREAKING:** Remove `VatConfig.platformConfig.fetch` — migrate to `globals: ['fetch', ...]` + `network.allowedHosts` ([#942](https://github.com/MetaMask/ocap-kernel/pull/942))
- **BREAKING:** `MakeAllowedGlobals` now takes a `{ logger }` options bag ([#942](https://github.com/MetaMask/ocap-kernel/pull/942))
- **BREAKING:** Type `VatConfig.globals` and `Kernel.make`'s `allowedGlobalNames` as `AllowedGlobalName[]` (a literal union) instead of `string[]`; unknown names are now rejected at the `initVat` RPC boundary ([#941](https://github.com/MetaMask/ocap-kernel/pull/941))
Expand Down
46 changes: 46 additions & 0 deletions packages/ocap-kernel/src/vats/SubclusterManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Mocked } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';

import type { KernelQueue } from '../KernelQueue.ts';
import { kser } from '../liveslots/kernel-marshal.ts';
import type { KernelStore } from '../store/index.ts';
import type {
VatId,
Expand Down Expand Up @@ -431,6 +432,51 @@ describe('SubclusterManager', () => {
bootstrapResult,
});
});

// Regression tests for #572 / #564. Previously, a vat that failed to build
// its root object caused `#launchVatsForSubcluster` to leak an unhandled
// rejection surfacing as a cryptic run-queue error, e.g.
// `SES_UNHANDLED_REJECTION: no record matching key 'queue.run.3'`. The
// bootstrap subroutines must now reject cleanly and roll back the
// subcluster instead.
describe('bootstrap failure propagation (#572)', () => {
it('rejects with the underlying error when the bootstrap message rejects', async () => {
const config = createMockClusterConfig();
// A vat failing to build its root object surfaces as the bootstrap
// delivery rejecting (Kernel.queueMessage deserializes the vat's
// CapData rejection into a plain Error before it reaches us).
(mockQueueMessage as ReturnType<typeof vi.fn>).mockRejectedValue(
new Error('vat failed to build root object'),
);

await expect(
subclusterManager.launchSubcluster(config),
).rejects.toThrow('vat failed to build root object');

// Not the old cryptic run-queue message.
await expect(
subclusterManager.launchSubcluster(config),
).rejects.not.toThrow('no record matching key');

// The subcluster is rolled back rather than left half-initialized.
expect(mockKernelStore.deleteSubcluster).toHaveBeenCalledWith('s1');
});

it('rethrows when the bootstrap message resolves to a serialized error', async () => {
const config = createMockClusterConfig();
// A serialized Error result (as a vat returns on a failed bootstrap)
// round-trips through kunser to an Error instance and is rethrown.
(mockQueueMessage as ReturnType<typeof vi.fn>).mockResolvedValue(
kser(new Error('bootstrap threw')),
);

await expect(
subclusterManager.launchSubcluster(config),
).rejects.toThrow('bootstrap threw');

expect(mockKernelStore.deleteSubcluster).toHaveBeenCalledWith('s1');
});
});
});

describe('terminateSubcluster', () => {
Expand Down
43 changes: 43 additions & 0 deletions packages/ocap-kernel/src/vats/VatManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,49 @@ describe('VatManager', () => {
);
expect(kref).toBe('ko1');
});

// #564: a vat that fails to launch (e.g. its `buildRootObject` throws,
// surfacing from `VatHandle.make`) must be attributed to the specific vat
// by both its kernel id and its ClusterConfig name, with the original
// error preserved as the cause.
it('attributes a launch failure to the vat by id and config name', async () => {
const config = createMockVatConfig();
const cause = new Error(
'Failed to initialize vat v1: buildRootObject threw',
);
makeVatHandleMock.mockRejectedValueOnce(cause);

await expect(vatManager.launchVat(config, 'bob', 's1')).rejects.toThrow(
'Failed to launch vat v1 (bob)',
);

// Downstream setup is skipped once the launch fails.
expect(mockKernelStore.initEndpoint).not.toHaveBeenCalled();
expect(mockKernelStore.setVatConfig).not.toHaveBeenCalled();
});

it('preserves the original error as the cause of a launch failure', async () => {
const config = createMockVatConfig();
const cause = new Error('buildRootObject threw');
makeVatHandleMock.mockRejectedValueOnce(cause);

const error = await vatManager
.launchVat(config, 'bob', 's1')
.catch((reason: unknown) => reason);

expect((error as Error).cause).toBe(cause);
});

it('omits the parenthetical when launched without a vat name', async () => {
const config = createMockVatConfig();
makeVatHandleMock.mockRejectedValueOnce(new Error('boom'));

const error = await vatManager
.launchVat(config)
.catch((reason: unknown) => reason);

expect((error as Error).message).toBe('Failed to launch vat v1');
});
});

describe('runVat', () => {
Expand Down
10 changes: 9 additions & 1 deletion packages/ocap-kernel/src/vats/VatManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,15 @@ export class VatManager {
if (subclusterId) {
this.#kernelStore.addSubclusterVat(subclusterId, vatName, vatId);
}
await this.runVat(vatId, vatConfig);
try {
await this.runVat(vatId, vatConfig);
} catch (error) {
// Attribute the failure (e.g. a vat whose `buildRootObject` throws) to
// the specific vat by both its kernel id and its ClusterConfig name, so
// the caller reports which vat failed rather than an opaque error.
const label = vatName ? `${vatId} (${vatName})` : vatId;
throw new Error(`Failed to launch vat ${label}`, { cause: error });
}
this.#kernelStore.initEndpoint(vatId);
const rootRef = this.#kernelStore.exportFromEndpoint(
vatId,
Expand Down
Loading