Create "safe" functions that return `Maybe a` instead of throwing an exception

[pjones]

I'm creating this ticket to help us track the work going on over at xmonad/xmonad-contrib#146.

Starting with the getWindowAttributes function, start creating versions that return Maybe a instead of throwing an exception. I'm not sure what the new naming scheme should be, but the original functions should just call the new version and throw if they get Nothing back.

[SOwOphie]

I'll look into this.

Grepping for foreign import ccall unsafe yields 303 results. Should I just slap something like

safely :: IO a -> IO (Maybe a)
safely a = either (const Nothing) Just <$> try a

safeGetWindowAttributes :: Display -> Window -> IO (Maybe WindowAttributes)
safeGetWindowAttributes d w = safely $ getWindowAttributes d w

around all of them or is there a way to narrow the results down by "notoriety"?

[pjones]

Awesome @SirBoonami, Thanks!

So, you have a good question. Can the calls into the C library produce exceptions? If so, then I like your approach. If not, it might be better to just get rid of the calls to throwIfZero.

We should also bring @geekosaur into this discussion. I wonder if we should even bother preserving the API. We could bump the version number to 2.0 and actually change the function signatures so we don't need unsafe/safe versions.

Thoughts?

[geekosaur]

A more useful check is to look for all occurrences of throwErrorIf and change them to produce Maybe instead.

[SOwOphie]

Good call. Including definitions and imports, there are currently 36 occurences of throwIf*. Changing these is a good start, we can take a look at foreign exceptions after that.

One more thing: All of these throwIf* calls supply error strings (like "xGetCommand returned status " ++ show status). Replacing these with a simple return Nothing discards this potentially helpful debug information. Wouldn't Either String a be a better choice?

[geekosaur]

I was actually thinking about that, either via Except or by logging to stderr (which you'll find in a few other places, some of which are annoying to other users of the X11 package); we might want to go with an `Except String`-based setup instead.

…

On Tue, Apr 11, 2017 at 2:12 PM, Felix Hirn ***@***.***> wrote: Good call. Including definitions and imports, there are currently 36 occurences of throwIf*. Changing these is a good start, we can take a look at foreign exceptions after that. One more thing: All of these throwIf* calls supply error strings (like "xGetCommand returned status " ++ show status). Replacing these with a simple return Nothing discards this potentially helpful debug information. Wouldn't Either String a be a better choice? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB8SoGvoJWJj2w5aKaFZjM9PKMm9SlgUks5ru8KrgaJpZM4M5Poh> .

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

[SOwOphie]

Not a fan of unconditional stderr logging either, even when tied to a compilation flag. Wouldn't want to drown the user in a flood of exception messages if he's looking for a specific one. Also, some exceptions naturally occur during normal production use, logging everything would needlessly inflate log files like ~/.xsession-errors for xmonad with useless clutter. Using a monad allows for much better control over what exactly get printed out.

Also, what is this Except thing you mentioned? Hoogle doesn't show anything useful...

[geekosaur]

http://hackage.haskell.org/package/mtl-2.2.1/docs/Control-Monad-Except.html

[SOwOphie]

Ah, thank you. I was thinking about EitherT from either myself (which is virtually the same thing). But I don't know if refactoring large parts of the lib to use a different monad, sprinkling liftIOs and runExceptTs all over the place is really worth the effort...

[geekosaur]

`EitherT` is deprecated; `ExceptT` is the replacement. Also, nobody said you had to use its `Monad` interface; how many people use `Either a`'s `Monad`?

…

On Tue, Apr 11, 2017 at 2:42 PM, Felix Hirn ***@***.***> wrote: Ah, thank you. I was thinking about EitherT from either <https://hackage.haskell.org/package/either> myself (which is virtually the same thing). But I don't know if refactoring large parts of the lib to use a different monad, sprinkling liftIOs and runExceptTs all over the place is really worth the effort... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB8SoI-kPZMGDP2fzKgH5gsvgk_CljV5ks5ru8mKgaJpZM4M5Poh> .

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

[SOwOphie]

So you mean our functions should look something like a -> IO (Except String b)? Because that's probably our best option imo...

[geekosaur]

Probably. `Either` would also work for that usage, but both the old `Either` and the `Error` instances were deprecated in order to make `Either` a simple product type that doesn't favor either alternative and `Except` the one that specifically considers its right type the normal one.

…

On Tue, Apr 11, 2017 at 3:11 PM, Felix Hirn ***@***.***> wrote: So you mean our functions should look something like a -> IO (Except String b)? Because that's probably our best option imo... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB8SoCU092GOawOaDPycM0-cvm639ZDhks5ru9B1gaJpZM4M5Poh> .

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net