search

metosin / sieppari

Small, fast, and complete interceptor library for Clojure/Script
Eclipse Public License 2.0
207 stars 21 forks source link

Handle JS async rejections #11

Closed arichiardi closed 5 years ago

arichiardi commented 6 years ago

Hi again! As part of my port to cljs I realized what could be an issue..maybe.

It seems like at the moment the continue protocol only accepts one callback, which makes sense most of the times, the only issue being js/Promise rejection.

One could add another callback for errors but I am not myself convinced it is the right thing to do. It would certainly handle this kind of situation that at the moment trigger an uncaught error warning.

Or maybe it is simply better to let it blow and fail fast.

Thoughts?

arichiardi commented 5 years ago

We are using sieppari - the cljs port - and we really are missing this.

In case of unexpected exception thrown in a promise there is nothing putting the :error in the context and what is worse is that node is then printing a terrible warning and not doing anything significant.

One solution to this would be to add a protocol function on the async context.

It cannot work only on this, because channel items would already be consumed. So my hunch would be to pass also the last item a/continue consumed.

arichiardi commented 5 years ago

Thinking more about this it probably does not make sense to touch the protocols.

Some helper around each AsyncContex could go a long way.

Need some more hammock time, any input is welcome :smile:

nilern commented 5 years ago

The problem seems to be that we don't catch async exceptions like we do sync ones. So when e.g. a rejected promise appears the Sieppari stack and queue get thrown away, the execution stops and you just get the rejected promise. We probably do have to add error handling functionality to the async protocol.