`attempt` doesn't handle exceptions in Aff function

[kika]

It appears that attempt is supposed to handle exceptions in the Aff function it runs, but it doesn't despite having a try ... catch... block in its FFI implementation. Repro case:

1. Checkout https://github.com/Thimoteus/purescript-simple-request.git
2. Change reddit.com to reddit.com:8088 at https://github.com/Thimoteus/purescript-simple-request/blob/master/test/Main.purs#L34
3. Run the test with pulp test

The program bails out with EPROTO exception (because there's nothing ready for SSL handshake at port 8088).

Expected result: Left Error from attempt with the corresponding exception.

[jdegoes]

The first place to look is here. Make sure any place which can cause an exception will call the onErr callback.

[kika]

@jdegoes Does this mean that functions in Aff indeed do not catch exceptions and we need to catch ourselves and call error callbacks? In Aff.js _attempt does use try...catch..., _runAff doesn't, _liftEff does again. I can't understand the system and difference.

[jdegoes]

@kika If you create your own Aff, you need to make sure that you always use the error callback in the event of an error. I am not sure why _attempt uses try / catch, probably we can delete that. The point is that if you create your own native Affs which are functioning correctly, then attempt will always "catch" the error (through the error callback). If that does not happen, then it's a bug in Aff and will be fixed right away (in this case, I am not sure, but the Javascript code does not look safe to me).

[kika]

@jdegoes ok, thanks, got it. So the takeaway is this:

1. We need to catch exceptions in our Affs and call error callbacks
2. The try... catch... in _attempt actually does anything only if aff() function is synchronous. I've read up on async exception handling in js and it seems that you can't catch the exception this way, so indeed this try/catch pair is not necessary. CC @Thimoteus

[natefaubion]

Aff now only uses try/catch in liftEff and makeAff. It's expected that anything else uses the error channels.