singpolyma / unexceptionalio

IO without any PseudoExceptions
http://hackage.haskell.org/package/unexceptionalio
ISC License
28 stars 5 forks source link

Remove fromIO' #12

Closed hanshoglund closed 6 years ago

hanshoglund commented 6 years ago

This is to argue fromIO' (with the tick) should be removed.

The fromIO and unsafeFromIO already provide a complete entry-point to the library: one safe and one performant (with proof obligation on the caller).

(Great work with this library, btw!)

singpolyma commented 6 years ago

fromIO' is intended for wrapping IO actions with known Exceptions (more specific than just SomeNonPseudoException) -- for example, many IO actions from the report will only throw IOException -- yes, you could try this manually and then use unsafeFromIO but why not have a helper for this fairly common case?

hanshoglund commented 6 years ago

My primary issue is really that fromIO' is unsafe and thus should be harder to reach.

With this library it is particularly important that people wrap their underlying IO's correctly, or UIO computations will break their guarantees, and thus become meaningless.

Note that even when wrapping a known library there is no way of knowing exactly what it might throw, as exceptions are (of course) not tracked in the types. They may also change between library versions without breaking APIs, making accidental breakage extremely likely if unsafe... functions are used to wrap exterenal APIs. Thus fromIO is the correct way to use this library in almost all cases.

We could still provide the unsafe functions, but they should be clearly marked as such as and come with big warnings against misuse. Something like:

-- | Wrap an 'IO' action. Normal/non-pseudo exceptions will be caught and reported using 'Left'.
fromIO :: Unexceptional m => IO a -> m (Either SomeNonPseudoException a)

-- | Wrap an 'IO' action throwing no exceptions but those contained in 'PseudoException'.
--
-- /Warning/ if the wrapped exception throws a non-pseudo exception, behavior of the resulting computation is undefined. Consider using `fromIO` instead.
unsafeFromIO :: (Unexceptional m) => IO a -> m a

E: Can provide a PR if you agree.

singpolyma commented 6 years ago

The docs for fromIO' already carry a warning -- but I could see renaming the function to include the word unsafe

hanshoglund commented 6 years ago

OK, will make another PR to that effect.

hanshoglund commented 6 years ago

Please review #14

singpolyma commented 6 years ago

New idea: change it to fromIO' :: (Exception e) => (SomeNonPseudoException -> e) -> IO a -> UIO (Either e a)

To recover the current behaviour, users can use fromIO' (error. show) or similar, but could also have an "Unknown" part of their error sum type and use the safer fromIO' (const Unknown)

hanshoglund commented 6 years ago

I like that idea! And if I'm not mistaken fromIO = fromIO' id (at least morally).