racket/file: check file-exists?

[bennn]

Check file-exists? before passing the file on to file-size and/or
open-input-file. The latter led to confusing error messages, e.g.:

(file->string "DNE")
;; file-size: cannot get size

affects: file->value file->list file->string file->bytes
file->lines file->bytes-lines

[mflatt]

I'm generally skeptical of checking whether a file exists before opening it, because there is a race after the check if some other process or thread can delete the file. Also, there may be a significant cost involved with the check on some filesystems.

I don't have a good answer for reporting the error as from file->string instead of file-size, though, and that's a general problem with non-contract errors.

[LiberalArtist]

Instead of checking first, what about intercepting the error and rewriting it? When I try this, here's what I get:

> (with-handlers ([values values])
    (file->string "DNE"))
(exn:fail:filesystem:errno
 "file-size: cannot get size\n  path: /Users/philip/DNE\n  system error: No such file or directory; errno=2"
 #<continuation-mark-set>
 '(2 . posix))

Racket seems to have all of the information necessary to reconstruct the kind of error—there's even an exn:fail:filesystem:exists, but it's documented as only coming from attempts to create a file—but I don't think there's a provided function to translate the values from an exn:fail:filesystem:errno to a string, for example.

[bennn]

Hm, ok.

The latest commit ignores errors from file-size in file->x and leaves everything else alone (42b66ea).

If the user sees an error, it'll say "cannot open input file" (similar to file->lines and others).

And if there's a race that creates the file after the call to file-size, then the function should now succeed.

[gus-massa]

Instead or rewriting the error, is it possible to make a version that has an argument with the name of the function to blame? Something like for/fold/derived.

[bennn]

Maybe, but I think that'd require a change to define/who --- which might come with its own problems.

I'd rather close this PR than try getting into that.

(But I would be happy to follow the error-rewriting approach if there's consensus that it's better; ignoring the file-size error seems best to me)

[samth]

Can you add an error message test that would have failed prior to this? Or at least post an example here?

[bennn]

I've added error-message tests.

My first comment has the key parts of an example, but here it is in full:

Example program

#lang racket
;; ./DNE must not exist on your filesystem
(file->string "DNE")

Previous error (v7.6)

file-size: cannot get size
  path: /Users/ben/code/racket/fork/DNE
  system error: No such file or directory; errno=2
  context...:
   raise-filesystem-error
   .../racket/file.rkt:763:0: file->x
   call-with-values
   call-in-empty-metacontinuation-frame
   body of "/Users/ben/code/racket/fork/file-string.rkt"
   for-loop
   run-module-instance!
   perform-require!
   namespace-require+
   #%for-each
   call-in-empty-metacontinuation-frame
   thunk_10
   call-in-empty-metacontinuation-frame
   call-with-empty-metacontinuation-frame-for-swap

Current error (42b66ea)

open-input-file: cannot open input file
  path: /Users/ben/code/racket/fork/DNE
  system error: No such file or directory; errno=2
  context...:
   raise-filesystem-error
   open-input-file
   .../private/kw-file.rkt:102:2: call-with-input-file*
   call-with-values
   call-in-empty-metacontinuation-frame
   body of "/Users/ben/code/racket/fork/file-string.rkt"
   for-loop
   run-module-instance!
   perform-require!
   namespace-require+
   #%for-each
   call-in-empty-metacontinuation-frame
   thunk_10
   call-in-empty-metacontinuation-frame
   call-with-empty-metacontinuation-frame-for-swap

This current error does not point to file->string (bad), but arises
without a call to file-exists? (good).

[pmatos]

ping! Shall we merge this then?