HPInc / HP-Digital-Microfluidics

HP Digital Microfluidics Software Platform and Libraries
MIT License
3 stars 1 forks source link

Add exception channel to macro interpreter execution. #100

Open EvanKirshenbaum opened 8 months ago

EvanKirshenbaum commented 8 months ago

Found this while looking at #99, and I really need to fix it.

When the macro lang execution raises a MaybeNotSatisfiedError or AlreadyDropError, what's supposed to happen is that the expression evaluation gets aborted, the error message gets printed, and everything goes on as normal.

The problem is that because the exception is actually raised inside the DevCommThread (as an action hung on a future), it isn't caught by anybody, a stack trace is printed, and the thread exits, essentially crashing the system.

I obviously realized that this would be a problem, because grepping through the code, nobody actually tries to catch the parent EvaluationError.

Looking at monitor.py, what it currently does is

 (interp.evaluate(expr, cache_as="last")
                    .then_call(lambda pair: print(f"  Interactive cmd val ({pair[0].name}): {pair[1]}")))

where interp is a DMFInterpreter whose evaluate() says

future = executable.evaluate(self.globals, required=required)

What I think I need to do is to have evaluate() take another parameter which is someplace that can stash an exception (which may not actually ever be really raised, unless I can find a place to put the try block) and which is passed from call to call, refusing to go further if an exception was seen. Then, at the end the exception can be printed.

This is still a bit hand-wavy, but it needs to be done for this language to be usable.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Apr 29, 2022 at 3:05 PM PDT.
EvanKirshenbaum commented 8 months ago

This issue was referenced by the following commits before migration:

EvanKirshenbaum commented 8 months ago

I'm not going to have it in the first iteration, but it would be nice if somewhere (perhaps in Environment), we kept enough information to print a macro-focused stack trace. Then we could have EvaluationError require one of these, to be printed out when the exception is noticed by the monitor.

Migrated from internal repository. Originally created by @EvanKirshenbaum on May 02, 2022 at 10:48 AM PDT.
EvanKirshenbaum commented 8 months ago

First things first, WorkerThread and the on_press() within BoardMonitor.dmf_lang_widgets() now catch RuntimeErrors and print them (with their Python-based stack traces). This isn't quite what we want for end-user mistakes, but at least it keeps the system from crashing.

Migrated from internal repository. Originally created by @EvanKirshenbaum on May 02, 2022 at 10:51 AM PDT.
EvanKirshenbaum commented 8 months ago

Okay, it all seems to work. Rather than having execute() take an extra parameter, I just propagate EvaluationError as a value. Whenever I see one, it gets pushed all the way to the end.

To support this, I added

MaybeError = Union[Val_, EvaluationError]

Unfortunately, there's now a lot of code that says essentially

if isinstance(arg, EvaluationError):
  return arg   # or Delayed.complete(arg)
else:
  # Do whatever you would've done before

Typically this is in new functions that replace what used to be simple lambda expressions. It would be nice to be able to have a couple of helper functions that take a Callable and check the arg. But some of these lambdas took more than one arg, and I'm not sure what to do with that.

Migrated from internal repository. Originally created by @EvanKirshenbaum on May 03, 2022 at 11:58 PM PDT.
EvanKirshenbaum commented 8 months ago

I added a couple of helper functions to simplify future chaining with error propagation:

def error_check(fn: Callable[..., Val_]) -> Callable[..., MaybeError[Val_]]:
    def check(*args) -> MaybeError[Val_]:
        first = args[0]
        if isinstance(first, EvaluationError):
            return first
        return fn(*args)
    return check

def error_check_delayed(fn: Callable[..., Delayed[Val_]]) -> Callable[..., Delayed[MaybeError[Val_]]]:
    def check(*args) -> Delayed[MaybeError[Val_]]:
        first = args[0]
        if isinstance(first, EvaluationError):
            return Delayed.complete(first)
        # Well, *that's* a kludge
        return fn(*args).transformed(lambda x: x)
    return check

The notion is that

future.transformed(fn)

becomes

future.transformed(error_check(fn))

and

future.chain(fn)

becomes

future.chain(error_check_delayed(fn))

As the comment in error_check_delayed() notes, in order for the type checking to work, we need to introduce a new future with an identity transform. I guess an alternative would be to simply say

return cast(Delayed[MaybeError[Val_]], fn(*args))

which is also a kludge, but more efficient. This is reasonable because we know that the Delayed object is always going to be posted "from the inside" as it were, so it's reasonable to assert covariance. What would be really useful (and I think I'll add an issue on it) would be to figure out how to distinguish between a read-only covariant future and a non-covariant (and possibly contravariant) future that has a post() member. This may be beyond the ability of Python's type annotations, though.

Migrated from internal repository. Originally created by @EvanKirshenbaum on May 04, 2022 at 11:11 AM PDT.