CrabDude / trycatch

An asynchronous domain-based exception handler with long stack traces for node.js
MIT License
246 stars 17 forks source link

re-implement on top of async listener #32

Closed Raynos closed 10 years ago

Raynos commented 10 years ago

0.12 async listener will allow implementing long stack traces without monkey patch hacks.

The async-listener shim on npm will give 0.10 support and "hide" all the monkey patch hacks.

CrabDude commented 10 years ago

TLDR; async-listener allows you to implement domains, not async try/catch

FWICT, everything will still need to be monkey patched, at the very least EventEmitter since it's a userland/core boundary issue, not an async source issue.

trycatch isn't just about long stack traces, it's primary focus is allowing for throw-safe code, which will always require wrapping the top-level abstraction (e.g., fs.readFile).

@trevnorris Am I wrong?

Raynos commented 10 years ago

@CrabDude If you can't implement try catch using async-listener then that sucks.

I would have expected you can just put try catches in there and everything would be good. Doesn't it allow you to intercept all event loop entries ?

trevnorris commented 10 years ago

@CrabDude We've actually pulled support for using AsyncListeners in domains. It became too complicated with how tightly coupled it all is. The point of AL is to allow you to know when an event is being constructed that will break the callstack, and then allow you to know when the callback from that async event is about to be called. Then also handle any errors that occur withing the execution chain of that callback.

I've written a patch specifically for EventEmitters here: https://github.com/joyent/node/pull/6502 The point of this was to allow similar functionality as AL with EE. It's all still being worked out.

As for something like fs.readFile(), it would be easy to capture any errors that occur within that callback, wrap up the resources (since you're directly passed the callers context) and continue on with your life.

trevnorris commented 10 years ago

After thinking this over, it occurred to me that trycatch is a top down approach. Whereas AsyncListener is bottom up. AL allows you to hook at the lowest level possible. Basically, at the transition point between libuv and V8.

AL sits on top of the AsyncWrap C++ class, from which all methods that have an asynchronous callback inherit. It allows you to capture absolutely everything. Including those things that don't have an associated JS API to monkey patch.

The API allows you to trigger a callback when the object is created, allowing you to capture state or whatever other information you prefer, call a function just before the return callback is fired, to help setup state, and a function when the callback is complete, to teardown that state.

The error callback is particularly useful because it allows you to capture any error since the beginning of the asynchronous stack. This is where AL and trycatch differ. The error will bubble completely through to the originating call. Within the error function you are given access to the context of the originating callback. This allows for better resource tear down and application recovery. You also have the option of returning whether the error was able to be handled or not. Allowing it to bubble further of needed. This sounds similar to domains, but the implementation is more robust and less intrusive.

This isn't a full set of what AL can do, but at least what I believe is applicable here. So unfortunately you can't just use this to wrap a try/catch around everything. Though in the example of fs.readFile() you would be able to do something like close the file descriptor and recover resources.

CrabDude commented 10 years ago

Yes. That's consistent with my understanding. AL would allow cleanup after the stack has been torn, like a try/catch at the top of the stack. Unfortunately, this requires specific knowledge of the executing code in order to perform proper cleanup.

One purpose of trycatch is to avoid this entirely by protecting core from userland errors, and thus any need for cleanup, and so unfortunately, trycatch cannot be implemented on top of AL.