Errors thrown in an Async#map don't always become Rejected

[flintinatux]

Describe the bug
If an error is thrown in an Async#map, it doesn't always become a Rejected.

To Reproduce

const { Async } = require('crocks')

Async((rej, res) => res('a'))
  .map(() => { throw new Error('bad') })
  .fork(err => console.error({ err }), val => console.log({ val }))

Expected behavior
I would expect an object shaped like { err } to be logged to console.error, but instead the error is thrown and logged by itself, and neither of the forked functions are called.

Additional context
If you wrap a Promise instead, it works as expected:

Async.fromPromise(() => Promise.resolve('a'))('b')
  .map(() => { throw new Error('bad') })
  .fork(err => console.error({ err }), val => console.log({ val }))

[evilsoft]

@flintinatux Sorry it took so long to get back to you sir.

I was going back through my notes and it looks like we did this by design. It has to do with Functor needing to map its source category one-to-one. So if something provides an Error in the source category of JavaScript, then that exception must be realized in the target category as well. While it is a right pain in the bum, it is required for keeping the laws straight.

If you need to trap errors, may I suggest moving from map to chain and take advantage of the resultToAsync Natural Transformation and utilize the tryCatch helper. That way with the Functor composition it will get you what you need and make the maths 😄.

Please let me know if you can think of a good argument against its current implementation that can also make the maths balance.

[flintinatux]

Thanks for taking a look at, @evilsoft. I understand the maths, so I think that's fine. The trouble is that the behavior doesn't match when the Async is created by wrapping a Promise-returning function. If you run the second example above, the thrown error is caught, and the Async becomes a Rejected. Which matches the error-handling behavior of a Promise, but not your description of an Async.

[evilsoft]

@flintinatux Oh. I totes agree with that. The way I justified that before, is that it we a Nat Transform from Promise -> Async and Promise in the Category of JS behaves that way. But maybe after thinking about your statement I see us of having (2) options here:

1. Keep it as is and provide better documentation to account for this case.
2. Re work it so that even Promises throw when maping. Could be a PITA, but would provide less surprise and more 🌽sistancy.

I have learn to trust the ol' @flintinatux gut, so do any of those options sparkle for you? Do you happen to have another suggestion?

[flintinatux]

The way it feels as a consumer of the library is that if I've got an Async in my hand, it should always behave the same regardless of how I made it. So if Async's aren't supposed to handle errors thrown in a .map(), then once I move from a Promise to an Async, it shouldn't handle errors anymore.

Which is a bummer, because in my head an Async caught errors and converted to a Rejected. Just a bad assumption on my part, I guess, probably carried over from one of the best parts of Promises.

[evilsoft]

So are you cool with this resolution or is there something more you think we should explore.

Funny thing the two things that make a Promise, not a Functor is what many people 🌽sider the best parts about them:

• They catch errors
• I cannot map to a nested Promise

These (2) things are some of the motivating reason that @puffnfresh create Fantasy Land. He was told by some of the spec submitters something along the lines of:

"Yeah this is really not happening. It totally ignores reality in favor of typed-language fantasy land"

quoted from: this discussion

and thus Fantasy Land was born!!

[flintinatux]

Maybe it's because I've spent most of my coding career in Javascript, but I find the development of the language fascinating/totally-cray. New features either drown in higher expectations than the original motivation (like Promises, meant to provide better async error-handling than callback hell, but not good enough because they aren't perfect monads), or they end up converted by third-party libraries into uses that don't even come close to the original spec (like react/babel/webpack's usage of import/export).

I could continue, but back on point: I'll almost always vote for consistency and fewer surprises. I submitted this issue because I got surprised and - as a result - confused. Whatever we can do to follow the maths rules AND not be unexpected, that will be a win in my book.

[puffnfresh]

Functor laws require this to be the implementation, if you follow the rules, it can't be unexpected behaviour. Also, we shouldn't use exceptions, ever.

[puffnfresh]

like Promises, meant to provide better async error-handling than callback hell, but not good enough because they aren't perfect monads

There's no such thing as "perfect" monads. Monad has a definition so something either is, or isn't. Promises could have been monadic, and they would have been far more useful. I believe this even more, 6 years later.

[flintinatux]

My apologies. I took the bait and wandered into opinion territory, and now we're off topic. I'll wander back.

The issue at hand is this:

• If I convert a Promise into an Async via Async.fromPromise, then errors thrown in a .map() are caught and converted to a Rejected.
• If I create an Async with the regular factory function, errors thrown in a .map() are not caught.

I find this mismatch in behaviour to be unexpected. Am I incorrect in feeling this way? If this should be expected behaviour, I guess at the very least I would want it documented to try and avoid surprises.

[evilsoft]

@flintinatux
I think you hit the nail on the head with the fromPromise function. I think this may be where the issue lies. So no matter what, this behavior will need to stay in place or it will invalidate Async as a Functor and all Natural transformations from other Sum Types also become invalid. Not to mention any Functor Compositions (Nested Functors is a way to view these)

So the reason it has to work this way is because in the Category of (gotta squint a little and ignore some bottoms) Javascript Types and Functions, Promises were created with this Error Handling in place. In order to provide a valid Functor, it MUST behave the same way in its source category when we get Async involved.

The issue is that we do not know the behavior when dealing with a Promise at the head or "chain"-ed in. Sometime it will throw, sometime it will not, depending on if fromPromise created the Async or not.

So the user of a crocks Async could alway treat it as it could blow and chain(tryCatch(fn)) one could never use map again without making sure it was also caught somehow, but I see how that may not be intuitive for the beginning/casual user.

Another option would be to somehow (don't know how yet, maybe with how the type is logged) signal to the user that a promise is in the mix.

But no matter where we go, I agree to start we need good documentation around the what and why of this behavior.

[flintinatux]

Ok. If this behavior is correct, and should be expected, then I think the appropriate action is just to document it. Documentation is foremost about education, and the crocks docs take that seriously, so I'm confident it'll turn out just fine.

As for me, I'll go ahead and start bending the spoon in my own mind, and change my expectations. Thanks again for helping me understand.

[dalefrancis88]

Should not have closed, docs work is still not done