Error Serialization Issue

[kumavis]

encountered when Endo weblet calls a remote function of endo daemon and an expected error occurs. results in error not reaching weblet

Error#2: Passed Error has extra unpassed properties

[kriskowal]

I’m hoping that @gibson042 or @erights know what this error means in general and how to overcome it. This seems like a case where the marshal layer could safely omit the extra unused properties of an error rather than fail to transit. I know at least in the OCapN context, we have a lot of open questions about the treatment of errors over CapTP.

[mhofman]

Marshall definitely errors on extra own properties. What are the environment details? I'm not aware of any that adds these properties as own.

[erights]

Marshall definitely errors on extra own properties

For a top-level error, it does not:

https://github.com/endojs/endo/blob/f746e996dfa827170b408ab276c1c438500c9ca1/packages/marshal/test/test-marshal-smallcaps.js#L103-L106

https://github.com/endojs/endo/blob/f746e996dfa827170b408ab276c1c438500c9ca1/packages/marshal/test/test-marshal-smallcaps.js#L114-L117

Marshal explicitly avoids calling ErrorHelper.assertValid when serializing top-level errors under the theory that, when encountering a malformed error, it is better to report the diagnostic carried by the malformed error than it is to report that this error is malformed. This issue reports a case where the malformed error is being reported instead, for some reason.

How can I reproduce this? Is there a stack trace from where this "extra unpassed properties" error was first made? IOW, who calls the assertValid at the bottom of the visible stacktrace in the screenshot above?

[erights]

This seems like a case where the marshal layer could safely omit the extra unused properties of an error rather than fail to transit.

Marshal is indeed already supposed to do that, for top-level errors. Indeed, the test case I quote above seems to demonstrate that it does do that.

[kumavis]

What are the environment details? @mhofman

This is endo pet daemon, the error originally occurred in a compartment in a worker

The extra properties are:

columnNumber,
fileName,
lineNumber,

as a workaround for my experiment I added these to packages/pass-style/src/error.js in ErrorHelper.assertValid

[kriskowal]

@erights @mhofman How would you folks feel if the marshaller accepted any instanceof Error as an error but reflected it over the wire as an ordinary, hardened object with a message and Symbol.for('passStyle') set to "error"? What complications would be necessary to make that sound?

[erights]

What about the name? What about error-id?

an ordinary, hardened object ... Symbol.for('passStyle') set to "error"

Being pedantic, by definition if the Symbol.for('passStyle') is set to something, that it is not an ordinary hardened object. For each pass-style, invariants must be enforced.

Altogether, I think I don't understand the question. What observable differences are you looking for?

[kriskowal]

Poke @kumavis, re @erights question above.

My general feeling is that we haven’t got a good story for treating errors over CapTP yet. I would like the in-band representation of an error to be something like an Error instance with a message and maybe some Open Telemetry style trace nonces, with an out-of-band representation capturing arbitrary fields that would get collected by an aggregator. The daemon is in a good position to aggregate these errors. The daemon or its mignons must be in a position to dictate the nonces. I’m not sure what this means for the pass-style of Error objects with extra properties. It might just be that Errors are not pass-by-copy and can only be carried by CapTP in the position of a promise rejection.

[erights]

The only parts of an error that should be serialized are those that obviously do not need to be further redacted. The most useful part of almost any error is the stack trace, which is also something that should usually be redacted from untrusted clients, i.e., CapTP counterparties. But this redacted info is desperately needed by the programmers who should be authorized to see it.

Thus, the info carried by an error needs to be split into that portion to be serialized vs that further portion to be sent to some aggregator. It then becomes a separate policy decision by the proprietor of that aggregator who to share that normally redacted info with.

I don't think any of this conflicts much with the notion that errors are pass-by-copy by captp encodings as they are now. The remaining hard issue would be to serialize something like the optional errorId supposedly already supported by @endo/marshal. Something like this is needed to correlate the error-as-captp-encoded with the full error info sent to the aggregator.

From a quick skim, I think this is all still consistent with https://github.com/endojs/endo/blob/master/packages/ses/src/error/README.md , even though the latter was last edited over two years ago.

Other pressures to change what gets serialized over captp:

• Errors are legitimately used by untrusted captp counterparties to render error diagnostics on non-programmer UIs. But our current errors were not designed for that purpose.

• Standard ES errors now have more structure. In particular, on their public API they have a tree of (approximately) "cause" errors. It would be nice for these to also get serialized over captp by default. But this possibly conflicts with the next pressure:

• errors as serialized over captp should be a basis for inter-language interoperation. Nicely though, the additional errors sent to an aggregator can be language specific, because aggregators only have much weaker pressures on inter-language interoperation.

• A programmer might wish to publicly attach other unredacted diagnostic info to errors that would be available to untrusted captp counter-parties, so that those counterparties can use it, for example, to render better error messages.

[kumavis]

in the context of my work building on top of the endo daemon, I have some backend code and some frontend code i'm developing. the frontend calls the backend via the endo daemon. due to a bug, the backend throws an error.

at present this error is censored, making debugging more difficult. my primary interest is a better developer experience.

[erights]

Censored by what? Are you logging the error to the console? Are you running with the default ses console (the "causal console")?

my primary interest is a better developer experience.

Certainly, a very close second place next to the primary of not losing security!

[erights]

Sorry, I answered in haste before rereading the thread to remind myself of the context. Better answer coming soon I hope.

[erights]

Ok, rereading the thread, I need to know more about "the context of my work...the backend throws an error"

Do you have a console log from each? What do you actually see in the error as unmarshalled? What are you missing that you want to see, or that you want developers to see?

Are you leaving the following marshal option set to its default, or are you overriding it? What about the other marshal options?

marshalSaveError = err =>
      console.log('Temporary logging of sent error', err)

Attn @dckc -- possibly about missing stack traces too?

[erights]

@kumavis ?