Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 208 forks source link

should liveSlots stop announcing failed vat deliveries? #726

Open warner opened 4 years ago

warner commented 4 years ago

Currently, when the kernel delivers a message to a vat (via dispatch.deliver or one of the dispatch.notify* resolution calls), if that delivery fails (i.e. the invocation of target.foo(args) raises an exception, or returns a Promise which is ultimately rejected), the liveSlots layer announces the Error message and traceback on stdout, just before informing the kernel (via syscall.reject). The vat which sent the message is notified as well, of course, as long as they attached a .then() error handler to their result Promise.

This is useful for application authors, when exceptions/rejections are truly exceptional. But when they're a normal part of the API, the extra announcement can be distracting. Especially when the operator of the SwingSet process is not an author, merely a user who wants to e.g. run a validator that happens to include a SwingSet handler.

Should we add a flag like --show-rejections and hide these messages otherwise? Or should we write the details to a logfile and only emit a non-scary line to stdout which provides a token or log-id with which the developer can find those details?

erights commented 4 years ago

(Repeating some points from our discussion)

The validator is interested in the correctness of the platform, not the correctness of the user code.

Authors of user code are interested in the correctness of their user code and everything it relies on --- both other user code and the platform.

Normal JS errors that are provoked by user code according to JS semantics do not indicate that there is anything wrong with the platform. They do often help authors of that user code or authors of user code that relies on that code to diagnostic bugs. These should propagate according to normal JS semantics plus the semantics of our distributed object system. They should also contribute to logs that these authors may read, where these logs contain diagnostic info that is inappropriate to reveal to programmatic callers. (Hence the separation made by the assert package.) Under normal conditions, there's no reason for validators to be bothered with the occurrences of these errors.

(thought not covered in today's discussion attn @Chris-Hibbert @michaelfig )

We should support a pattern where the spawner of a vat can provide a virtual console or console-like logging abstraction, for use by assert among others, where the spawner has access to the info logged to that logging abstraction. From inside the vat, we should maintain the brilliant feature of the console logging abstraction that it is write-only, and so is not a communications channel among the objects in the vat.

Once we get our error-repair of the SES-shim working and integrated, this applies to stack traces as well. Programatic catchers of errors or handlers of rejected promise reasons should not be able to see those stack traces. However, when those error objects are given to the logger, the logger can (and should) log the stack trace as diagnostic info available to whoever can read that log.

Chris-Hibbert commented 4 years ago

It should be straightforward to add access to this kind of log to the admin interface supported by vat spawning, and I think that's the right place for it to go.

warner commented 4 years ago

Hm, so perhaps the vat owner (whoever has access to the new vat's admin facet) could subscribe to hear about all unhandled exceptions? Or could configure the new vat to record these instead of silently throwing them away, to be queried later? Sounds like the beginnings of a debug interface, cool.

warner commented 4 years ago

Oh, what about when the bootstrap() call fails? This message is synthesized by the kernel when the system starts up, so it has no result= Promise. If something goes wrong in the first (or non-first) turn of this initial crank, there is no vat waiting on the result. I think we'll need to handle this one specially, and print a message to the console (perhaps even aborting the entire SwingSet machine) if an error occurs.