Throw TypeError instead of terminating the simulation #985
Conversation
| """Example of trying to fork a coroutine that isn't a coroutine""" | ||
| yield Timer(500) | ||
| cocotb.fork(function_not_a_coroutine()) | ||
| yield Timer(500) |
There was a problem hiding this comment.
This test was uninteresting, the error happened before fork was even called.
Moved it to below, where it is interesting
| @@ -709,18 +707,12 @@ def schedule(self, coroutine, trigger=None): | |||
|
|
|||
| try: | |||
| result = self._trigger_from_any(result) | |||
| except TypeError: | |||
| msg = ("Coroutine %s yielded something the scheduler can't handle" | |||
There was a problem hiding this comment.
No need to include the name of the coroutine in the message any more, it appears in the stack trace, along with the line that performed the bad yield.
eric-wieser
force-pushed
the
bad-yield-as-exception
branch
from
2a4ead5
to
330971a
Compare
eric-wieser
changed the title
eric-wieser
force-pushed
the
bad-yield-as-exception
branch
from
330971a
to
0f3a88f
Compare
There was a problem hiding this comment.
LGTM with a couple of nits in the error messages to make them even more useful to the user. This is a great improvement to usability, thanks a lot for this @eric-wieser!
Previously, doing all of these things would cause the simulation to terminate: * `yield bad_obj` * `cocotb.fork(bad_obj)` * Invoking a function decorated with `@cocotb.coroutine` that does not produce a generator (typically: contains no yields) * A trigger failing to prime Instead, these operations now throw an exception at the point where they occur, meaning the user gets a proper stack trace, and can still perform cleanup in a try / finally. This extends `NullTrigger` to allow its result to be overriden, which is used here to throw an exception at a yield point.
eric-wieser
force-pushed
the
bad-yield-as-exception
branch
from
0f3a88f
to
f77936a
Compare
|
Updated with a second commit that improves the affected error messages. |
| buff = StringIO(buff_bytes.getvalue().decode("UTF-8")) | ||
| result.stderr.write(buff.getvalue()) | ||
| raise result | ||
| return RunningCoroutine(self._func(*args, **kwargs), self) |
There was a problem hiding this comment.
Coming in a future PR: Making the same simplification to test.__call__
eric-wieser
force-pushed
the
bad-yield-as-exception
branch
from
f77936a
to
8bbe12a
Compare
|
You tricked me, I need to update the tests now too |
eric-wieser
force-pushed
the
bad-yield-as-exception
branch
from
8bbe12a
to
530194c
Compare
|
Thanks for going the extra mile on this! |
Previously, doing all of these things would cause the simulation to terminate:
yield bad_objcocotb.fork(bad_obj)@cocotb.coroutinethat does not produce a generator (typically: contains no yields)Instead, these operations now throw an exception at the point where they occur, meaning the user gets a proper stack trace, and can still perform cleanup in a try / finally.
This extends
NullTriggerto allow its result to be overriden, which is used here to throw an exception at a yield point.