Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround #2491 at all call sites #2492

Merged
merged 2 commits into from
Aug 16, 2016
Merged

Workaround #2491 at all call sites #2492

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0c30b7
Workaround #2491 at all call sites
Blaisorblade Aug 14, 2016
e217cfb
decodeFileEither: propagate IOExceptions correctly
Blaisorblade Aug 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Data/Yaml/Extra.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
{-# LANGUAGE ScopedTypeVariables #-}
-- | Wrappers for Yaml functions to workaround
-- https://github.com/commercialhaskell/stack/issues/2491.
-- Import Data.Yaml.Extra in place of Data.Yaml to use this workaround.
-- Beware these functions construct/deconstruct the entire file at once!
module Data.Yaml.Extra (decodeFileEither, encodeFile, module Data.Yaml) where

import Control.Exception
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double-check this — I think it's OK (this is a straightforward synchronous exception, handled in the IO monad) but I'm not yet fluent with Haskell exceptions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much experience with Haskell exceptions either. Pinging @mgsloan.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since he seems busy, I'll trust my (more educated) judgement, assume this is fine, and merge this. Details below but feel free to skip.

Based on https://github.com/commercialhaskell/haskelldocumentation/blob/master/content/exceptions-best-practices.md, having read some more docs (including for MonadCatch, which doesn't apply here), having read the papers on Haskell exceptions (including async ones, also not applying here*), I think this is fine.

*We risk catching any async IOException, and I understand that's bad. But IOException shouldn't be used for asynchronous exceptions, and even safe-exceptions won't help there.
https://www.fpcomplete.com/blog/2016/06/announce-safe-exceptions

import Data.Yaml hiding (decodeFileEither, encodeFile)
import qualified Data.ByteString as B
import System.IO

-- Note: we refrain from using 'B.readFile' and 'B.writeFile', as they open
-- the file in binary mode rather than text mode.
decodeFileEither :: FromJSON a => FilePath -> IO (Either ParseException a)
decodeFileEither path = withFile path ReadMode $
decodeFileEither path =
handle (\(e :: IOException) -> return . Left . OtherParseException . SomeException $ e) $
withFile path ReadMode $
\hnd -> do
fileContent <- B.hGetContents hnd
return $ decodeEither' fileContent
Expand Down