3

In the following:

import Data.Bifunctor
import qualified Data.ByteString.Lazy.UTF8 as BLU

safeReadFile :: FilePath -> ExceptT Text IO Text
safeReadFile p = (lift $ doesFileExist p) >>= bool (throwError "File does not exist") (lift $ pack <$> readFile p)

safeDecodeJSONFile :: FromJSON a => Text -> FilePath -> ExceptT Text IO a
safeDecodeJSONFile t f = do
  contents <- safeReadFile f
  tryRight $ first (\x -> pack (x ++ (unpack t))) (eitherDecode (BLU.fromString (unpack contents)))

When I run runExceptT $ safeDecodeJSONFile "something" "nonExistantFile.json" I expect to get Left "Does not exist something" but instead I just get Left "Does not exist" - I know that the function I pass to first is being executed, since without the pack GHC complains that the type of (eitherDecode (BLU.fromString (unpack contents))) is ExceptT String IO a instead of ExceptT Text IO a - so why doesn't the concatenation from ++ also happen?

Cameron Ball
  • 4,048
  • 6
  • 25
  • 34
  • 1
    Just because it type checks doesn't mean it's executed. `safeReadFile f` is probably returning an error value and short-circuiting the computation. – 4castle Aug 02 '19 at 13:47
  • Yeah... you're right. In hindsight that's obvious. Post it as an answer and I'll be happy to accept it :D – Cameron Ball Aug 02 '19 at 14:02

1 Answers1

2

You've written

safeDecodeJSONFile t f = do
  contents <- safeReadFile f
  tryRight $ ...

The Monad instance for ExceptT gives up as soon as it hits Left, returning exactly that. So the tryRight ... never happens. You need to handle the Left case explicitly, perhaps using catchError.

While we're at it, there's still a problem. You write

safeReadFile :: FilePath -> ExceptT Text IO Text
safeReadFile p = (lift $ doesFileExist p) >>= bool (throwError "File does not exist") (lift $ pack <$> readFile p)

Unfortunately, this isn't reliable. First off, the file not existing is only one reason reading it can fail--there could be permission errors, network problems for networked filesystems, device errors if the file isn't a regular file, etc. Second, someone else could delete the file between the time you check its existence and the time you try to read it. The usual advice when trying to deal with files is not to check first. Just read the file and catch any exceptions using catch or similar in Control.Exception or wrappers around them

dfeuer
  • 48,079
  • 5
  • 63
  • 167