Skip to content

Commit

Permalink
fix: Correlate sent errors with received errors
Browse files Browse the repository at this point in the history
  • Loading branch information
erights authored Dec 15, 2020
1 parent 4ffb911 commit 73b9cfd
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 18 deletions.
4 changes: 3 additions & 1 deletion packages/SwingSet/src/kernel/deviceSlots.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ export function makeDeviceSlots(
return slotToVal.get(slot);
}

const m = makeMarshal(convertValToSlot, convertSlotToVal);
const m = makeMarshal(convertValToSlot, convertSlotToVal, {
marshalName: `device:${forDeviceName}`,
});

function PresenceHandler(importSlot) {
return {
Expand Down
4 changes: 3 additions & 1 deletion packages/SwingSet/src/kernel/initializeKernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ export function initializeKernel(config, hostStorage, verbose = false) {
throw Error('bootstrap got unexpected pass-by-presence');
}

const m = makeMarshal(convertValToSlot);
const m = makeMarshal(convertValToSlot, undefined, {
marshalName: 'kernel:bootstrap',
});
const args = harden([vatObj0s, deviceObj0s]);
// queueToExport() takes kernel-refs (ko+NN, kd+NN) in s.slots
const rootSlot = makeVatRootObjectSlot();
Expand Down
4 changes: 3 additions & 1 deletion packages/SwingSet/src/kernel/liveSlots.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
}

// eslint-disable-next-line no-use-before-define
const m = makeMarshal(convertValToSlot, convertSlotToVal);
const m = makeMarshal(convertValToSlot, convertSlotToVal, {
marshalName: `liveSlots:${forVatID}`,
});

const {
makeVirtualObjectRepresentative,
Expand Down
7 changes: 4 additions & 3 deletions packages/SwingSet/test/test-exomessages.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ async function testFailure(t) {
failureHappened = true;
t.is(
e.message,
'kernel panic kp40.policy panic: rejection {"body":"{\\"@qclass\\":\\"error\\",\\"name\\":\\"Error\\",\\"message\\":\\"gratuitous error\\"}","slots":[]}',
'kernel panic kp40.policy panic: rejection {"body":"{\\"@qclass\\":\\"error\\",\\"name\\":\\"Error\\",\\"message\\":\\"gratuitous error\\",\\"errorId\\":\\"error:liveSlots:v1#1\\"}","slots":[]}',
);
}
t.truthy(failureHappened);
t.is(controller.kpStatus(controller.bootstrapResult), 'rejected');
t.deepEqual(controller.kpResolution(controller.bootstrapResult), {
body: '{"@qclass":"error","name":"Error","message":"gratuitous error"}',
body:
'{"@qclass":"error","name":"Error","message":"gratuitous error","errorId":"error:liveSlots:v1#1"}',
slots: [],
});
}
Expand Down Expand Up @@ -122,7 +123,7 @@ test('extra message rejects', async t => {
t,
'reject',
'rejected',
'{"@qclass":"error","name":"Error","message":"gratuitous error"}',
'{"@qclass":"error","name":"Error","message":"gratuitous error","errorId":"error:liveSlots:v1#1"}',
[],
);
});
1 change: 1 addition & 0 deletions packages/captp/lib/captp.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export function makeCapTP(ourId, rawSend, bootstrapObj = undefined, opts = {}) {
convertValToSlot,
// eslint-disable-next-line no-use-before-define
convertSlotToVal,
{ marshalName: `captp:${ourId}` },
);

const valToSlot = new WeakMap(); // exports looked up by val
Expand Down
3 changes: 3 additions & 0 deletions packages/dapp-svelte-wallet/api/src/lib-dehydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ export const makeDehydrator = (initialUnnamedCount = 0) => {
const { serialize: dehydrate, unserialize: hydrate } = makeMarshal(
convertValToName,
convertNameToVal,
{
marshalName: 'hydration',
},
);
return harden({
hydrate,
Expand Down
4 changes: 3 additions & 1 deletion packages/dapp-svelte-wallet/api/src/lib-wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ export function makeWallet({
// Instead of { body, slots }, fill the slots. This is useful for
// display but not for data processing, since the special identifier
// @qclass is lost.
const { unserialize: fillInSlots } = makeMarshal(noOp, identitySlotToValFn);
const { unserialize: fillInSlots } = makeMarshal(noOp, identitySlotToValFn, {
marshalName: 'wallet',
});

/** @type {NotifierRecord<OfferState[]>} */
const {
Expand Down
31 changes: 23 additions & 8 deletions packages/eventual-send/src/track-turns.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ export const trackTurns = funcs => {
if (typeof globalThis === 'undefined' || !globalThis.assert) {
return funcs;
}
const { details: d } = assert;

hiddenCurrentEvent += 1;
const sendingError = new Error(
`Event: ${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
if (hiddenPriorError !== undefined) {
assert.note(sendingError, assert.details`Caused by: ${hiddenPriorError}`);
assert.note(sendingError, d`Caused by: ${hiddenPriorError}`);
}

return funcs.map(
Expand All @@ -54,13 +56,26 @@ export const trackTurns = funcs => {
hiddenCurrentTurn += 1;
hiddenCurrentEvent = 0;
try {
return func(...args);
} catch (err) {
assert.note(
err,
assert.details`Thrown from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
throw err;
let result;
try {
result = func(...args);
} catch (err) {
if (err instanceof Error) {
assert.note(
err,
d`Thrown from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
}
throw err;
}
// Must capture this now, not when the catch triggers.
const detailsNote = d`Rejection from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`;
Promise.resolve(result).catch(reason => {
if (reason instanceof Error) {
assert.note(reason, detailsNote);
}
});
return result;
} finally {
hiddenPriorError = undefined;
}
Expand Down
28 changes: 27 additions & 1 deletion packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,17 @@ const defaultSlotToValFn = (x, _) => x;
export function makeMarshal(
convertValToSlot = defaultValToSlotFn,
convertSlotToVal = defaultSlotToValFn,
{ marshalName = 'anon-marshal' } = {},
) {
assert.typeof(marshalName, 'string');
// Ascending numbers identifying the sending of errors relative to this
// marshal instance.
let errorCount = 0;
const nextErrorId = () => {
errorCount += 1;
return `error:${marshalName}#${errorCount}`;
};

/**
* @template Slot
* @param {Passable} val
Expand Down Expand Up @@ -592,10 +602,20 @@ export function makeMarshal(
// summary. If we do that, we could allocate some random
// identifier and include it in the message, to help
// with the correlation.

const errorId = nextErrorId();
assert.note(val, d`Sent as ${errorId}`);
// TODO we need to instead log to somewhere hidden
// to be revealed when correlating with the received error.
// By sending this to `console.log`, under swingset this is
// enabled by `agoric start --reset -v` and not enabled without
// the `-v` flag.
console.log('Temporary logging of sent error', val);
return harden({
[QCLASS]: 'error',
name: `${val.name}`,
message: `${val.message}`,
errorId,
});
}
case REMOTE_STYLE: {
Expand Down Expand Up @@ -720,7 +740,13 @@ export function makeMarshal(
);
}
const EC = getErrorConstructor(`${rawTree.name}`) || Error;
return ibidTable.register(harden(new EC(`${rawTree.message}`)));
const error = harden(new EC(`${rawTree.message}`));
ibidTable.register(error);
if (typeof rawTree.errorId === 'string') {
// errorId is a late addition so be tolerant of its absence.
assert.note(error, d`Received as ${rawTree.errorId}`);
}
return error;
}

case 'slot': {
Expand Down
6 changes: 6 additions & 0 deletions packages/marshal/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,15 @@
* @callback MakeMarshal
* @param {ConvertValToSlot=} convertValToSlot
* @param {ConvertSlotToVal=} convertSlotToVal
* @param {MakeMarshalOptions=} options
* @returns {Marshal}
*/

/**
* @typedef MakeMarshalOptions
* @property {string=} marshalName
*/

// /////////////////////////////////////////////////////////////////////////////

/**
Expand Down
6 changes: 4 additions & 2 deletions packages/marshal/test/test-marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ test('serialize static data', t => {
emptyem = harden(e);
}
t.deepEqual(ser(emptyem), {
body: '{"@qclass":"error","name":"Error","message":""}',
body:
'{"@qclass":"error","name":"Error","message":"","errorId":"error:anon-marshal#1"}',
slots: [],
});

Expand All @@ -81,7 +82,8 @@ test('serialize static data', t => {
em = harden(e);
}
t.deepEqual(ser(em), {
body: '{"@qclass":"error","name":"ReferenceError","message":"msg"}',
body:
'{"@qclass":"error","name":"ReferenceError","message":"msg","errorId":"error:anon-marshal#2"}',
slots: [],
});

Expand Down

0 comments on commit 73b9cfd

Please sign in to comment.