Fix base/System.IO.withFile to only annotate own exceptions

[ch1bo]

Why

withFile annotating any exception raised within the wrapped action is misleading when debugging. It makes users think that something is wrong with the file opened, although another IOException was raised. This is particularly confusing if "not found" errors of other files are raised, e.g. trying to start a process which is not found on the PATH.

What

withFile should not annotate / change unrelated exceptions.

For example, given the file test.hs:

import System.IO
main = withFile "test.txt" WriteMode $ \_ -> fail "test

when run with runhaskell test.hs, produces

test.hs: test.txt: withFile: user error (test)

while it should rather exit and print:

test.hs: user error (test)

How

• Upstream issue opened by @ndmitchell https://gitlab.haskell.org/ghc/ghc/-/issues/20886
• A merge request fixing this https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11866

[hasufell]

Since no API is changed, I guess this doesn't need an impact assessment? Although I expect that some tests examining error messages could break. I'm not sure we require running tests on clc-stackage (and afair that's problematic anyway).

[hasufell]

I backported the change to 9.6, so if anyone wants to try to run some selective tests:

ghcup install ghc -u "https://gitlab.haskell.org/api/v4/projects/933/jobs/1746054/artifacts/ghc-x86_64-linux-fedora33-release.tar.xz" clc237

[Bodigrim]

Thanks @ch1bo! Could you please copy the proposal here and update the top post? It's easier to discuss things when it's in the same place.

[ch1bo]

@Bodigrim Done and also updated the upstream MR based on your comments.

[hasufell]

What is the behavior of withFile fp WriteMode hGetContents?

I think it will now show filedescriptor information in ioe_filename: haskell/file-io#18 (comment)

Is that less or more confusing?

[hasufell]

$ ghci-clc237
GHCi, version 9.6.2.20240106: https://www.haskell.org/ghc/  :? for help
ghci> import System.IO
ghci> withFile "test" WriteMode hGetContents
*** Exception: test: hGetContents: illegal operation (handle is not open for reading)

It seems the base version of withFile is not affected since it doesn't use fdToHandle as described here.

[Bodigrim]

This looks simple enough and impactful enough to proceed with a vote sooner than usual.

Dear CLC members, let's vote on the proposal to amend withFile and withFileBlocking so that they annotate only their own exceptions. Current behaviour is confusing and prone to mask the actual cause of the exception or even overwrite the relevant filename, which makes debugging unnecessary painful. The original issue is available at https://gitlab.haskell.org/ghc/ghc/-/issues/20886 and the MR is at https://gitlab.haskell.org/ghc/ghc/-/issues/20886. This is not a breaking change, just a bug fix for incorrect behaviour.

@hasufell @mixphix @angerman @parsonsmatt @tomjaguarpaw @velveteer

---

+1 from me. This dovetails nicely into our recent efforts to improve UX of IO exceptions.

[hasufell]

+1

[velveteer]

+1

[tomjaguarpaw]

+1

---

This seems like an improvement because the old message was rather confusing, that is, it's more confusing to see

test.hs: test.txt: withFile: user error (test)

than

test.hs: user error (test)

because the connection to the withFile block is not explained. On the other hand the former contains more information. I wonder if it would be better than both to have something like

test.hs: Whilst in the withFile block for test.txt: user error (test)

I'd would be in support of that, if anyone wanted to make a follow-up proposal.

[hasufell]

@tomjaguarpaw IO actions that use the file-handle inside the withFile block and fail should already print the proper filepath the handle is related to. I'm not sure it's relevant how the handle was obtained.

For IO actions unrelated to the file handle, I don't think we need any more information.

[tomjaguarpaw]

Possibly. My suggestion would be to "piggyback" on the withFile block as a source of information about the origin of the error. Perhaps that's too "cute" or perhaps it's helpful. I tend to think the latter, but there are good arguments for the former.

[tomjaguarpaw]

I'm also confused why this change isn't also made for withBinaryFile (and to confuse the matter further, I think withBinaryFiles Haddock is wrong: https://gitlab.haskell.org/ghc/ghc/-/issues/24350).

[hasufell]

I'm also confused why this change isn't also made for withBinaryFile (and to confuse the matter further, I think withBinaryFiles Haddock is wrong: https://gitlab.haskell.org/ghc/ghc/-/issues/24350).

That seems to be an oversight. Please comment on the MR.