status-im / nim-chronos

Chronos - An efficient library for asynchronous programming
https://status-im.github.io/nim-chronos/docs/chronos
Apache License 2.0
352 stars 51 forks source link

Fixes compilation issues in v3 compatibility mode (`-d:chronosHandleException`) #545

Closed gmega closed 2 weeks ago

gmega commented 1 month ago

fixes #544

This PR adds the missing await calls which fixes the issue I was experiencing without visible ill effects. I've also added (somewhat naively) the ability to run the test suite in compat mode, though not sure this is a desirable change.

arnetheduck commented 1 month ago

Hm, this looks like it's cycling down the wrong path. Let's revert to what handleExceptions is supposed to do: remap Exception to AsyncExceptionError - this means that for any function where handleExceptions is enabled and it has raises, the raises list also needs to include AsyncExceptionError - of course, in legacy code this is not a problem (because async without raises allows any exception to pass), but the failing tests are likely failing because of this "extra" exception being raised - the solution is to write the tests such that when hndleExceptions is globally enabled, they have a different raises list

arnetheduck commented 1 month ago

hm, or .. maybe you're right, maybe it doesn't make sense for the global flag to apply to raises-annotated functions in which case the documentation needs to be updated as well.

This makes more sense in the context of upgrading an existing codebase with mixed annotated and non-annotated code - let's go with your proposal, forget my previous comment.

gmega commented 1 month ago

Yeah, even though my proposal actually comes from my understanding of what I thought was your proposal ( :smile: ), I came to realize that being able to get incremental exception checking as I annotate a legacy codebase doesn't seem bad at all. Gonna take a stab at updating the docs.

gmega commented 1 month ago

OK, added a tentative update to the docs.

gmega commented 3 weeks ago

@arnetheduck ping :slightly_smiling_face: