purescript-node / purescript-node-fs

Node.js file I/O for purescript
MIT License
33 stars 34 forks source link

Errors in some APIs can't be handled #42

Closed cprussin closed 6 years ago

cprussin commented 6 years ago

For example, in Node.FS.Stream, createReadStream has the type:

createReadStream :: FilePath -> Effect (Readable ())

But the Node API can raise an exception, for instance, if the file doesn't exists. Likely the type should be:

createReadStream :: FilePath -> Effect (Either Error (Readable ()))

Is there any other solution that would be preferred here over catching the exception and returning an Either?

hdgarrood commented 6 years ago

I think it's best left as-is, because the current type best matches the Node API. If downstream libraries want to provide a more ergonomic interface that's fine, but the policy for the libraries in this org is that we try to wrap the node APIs as closely and in a minimally opinionated manner, so that they remain useful for as many use cases as possible.

hdgarrood commented 6 years ago

I don't think it's accurate to say that errors can't be handled, by the way: purescript-exceptions gives you the tools to handle these errors.

cprussin commented 6 years ago

Ah interesting, so I didn't actually know that you could use purescript-exceptions like that--but I guess that I should have, since it's the same way it was used before the removal off the Eff row. That's good information, thank you!

So I actually still think that this is a good change to make, for a few reasons. This is likely not the right place for this discussion, since it has implications about patterns for purescript-node in general, but here goes:

1) It's not about ergonomics, it's about type safety. Standard Node bindings in PureScript should retain type safety. Without the Eff row type, there is now no type safety around the possibility of exception.

2) Previously, with the Eff row type, it became obvious which APIs could throw exceptions. Now, it is nonobvious. In the local scope, this isn't really a big problem, since devs likely should know which Node APIs have exceptions and as you mention, folks can use purescript-exceptions. Where it becomes a problem is that it's very easy for folks to consume purescript-node libs in their code and not handle exceptions, leading to a bunch of libraries that may throw exceptions, but have no indication that they will, unless folks trace the entire code stack down to the purescript-node calls. This defeats a huge selling point of purescript (and pure languages in general)--that being that the type system clearly indicates errors and forces them to be handled. I believe it should be the responsibility of purescript-node bindings to indicate when an API may throw errors, at the very least by naming it something like unsafe, but preferably through the type system itself.

3) At least in Haskell, and I would assume PS as well, Either Error a is the conventional return type of something that might have an error. I would expect a binding to an API that might return an error to use the conventional return type for errors. If in PS the conventional return type is indeed Either Error a, then I don't think that making that mapping is either adding unnecessary distance from the underlying API or adding unnecessary opinion (my viewpoint here is that handling exceptions with purescript-exceptions is already an opinion on how exceptions should be handled, and it conflicts with the more conventional way of handling exceptions in pure languages).

Like I said, it's probably not the right place for this discussion, since it's more of a philosophical question about how exceptions are handled in purescript-node in general. Let me know if there's a better place to raise the question and I'm happy to move it. For this specific issue, I'm happy to table it and resolve it later based on the outcome of the more general discussion.

hdgarrood commented 6 years ago

This is not a bad place for this discussion, although perhaps Discourse might be better.

The issue of whether IO actions should indicate explicitly in their types whether they throw exceptions and what kind of exceptions they throw is a hotly contested issue with plenty of previous discussion in the Haskell world, but there are (imo) strong arguments in favour of the current API. See for example https://www.fpcomplete.com/blog/2016/11/exceptions-best-practices-haskell and the things it links to. (Note that IO in Haskell is a little bit different from our Effect, but I think the same arguments apply in this case.)

I also don't agree that Either Error a is the conventional return type of something that might have an error, in either Haskell or PureScript. Not only is "this might fail" part of the contract of IO and of Effect (see above), but also in code which doesn't concern itself with IO you have plenty of options:

and probably others.

cprussin commented 6 years ago

That's a great article. So if I'm understanding correctly, basically the idea is, explicit errors are unnecessary in the type signature because the contract of IO (and Effect) is that it always carries the possibility of exception? That makes sense, and many thanks for the pointers.

hdgarrood commented 6 years ago

Welcome :) Yes, that’s exactly it.