[m-rev.] for review: Introduce io.system_error to io.m public interface.

[Julien Fischer]

Hi Peter,

On Mon, 22 Aug 2022, Peter Wang wrote:

> Implement the error handling proposals from February and August 2022
> on the mercury-users list.

Actually, the August discussion was on the reviews list.

> We add io.system_error to the public interface of io.m
> and document what its foreign representation is for each backend.
>
> We allow io.error to optionally contain an io.system_error value,
> and provide predicates to retrieve the io.system_error from an io.error.
> The user may then inspect the system error via foreign code.
>
> We also provide a predicate that takes an io.error and returns a name
> for the system error it contains (if any). This makes it relatively easy
> for Mercury programs to check for specific error conditions.
>
> By returning platform-specific (actually, implementation-dependent)
> error names, we are pushing the responsibility of mapping strings to
> error conditions onto the application programmer. On the other hand, it
> is not practical for us to map all possible system-specific error codes
> to some common set of values. 
> We could do it for a small set of common error codes/exceptions, perhaps.

I would argue the *if* such errors can be detected across all of our target
platforms, then we should just have more specific result types for the
operations that produce them.

> The standard library will construct io.error values containing
> io.system_errors. However, we do not yet provide a facility for user
> code to do the same.

...

> diff --git a/library/io.m b/library/io.m
> index acf43f53e..0123f8cb1 100644
> --- a/library/io.m
> +++ b/library/io.m
> @@ -130,7 +130,20 @@
>     ;       eof
>     ;       error(string, int). % error message, line number
> 
> -:- type io.error.   % Use error_message to decode it.
> +    % A value indicating an error.

       an I/O error.

> +    % This may or may not have an associated io.system_error value.
> +    %
> +:- type io.error.
> +
> +    % A system-dependent error value.
> +    % For C backends, this is either an errno value (e.g. ENOENT)
> +    % or a Windows system error code (e.g. ERROR_FILE_NOT_FOUND).
> +    % For the Java and C# backends, this is an exception object.
> +    %
> +:- type system_error.
> +:- pragma foreign_type(c, system_error, "MR_Integer").

You can set can_pass_as_mercury_type on that.
(The existing code should have done that.)

> +:- pragma foreign_type("C#", system_error, "System.Exception").
> +:- pragma foreign_type(java, system_error, "java.lang.Exception").
>
>     % whence denotes the base for a seek operation.
>     %   set - seek relative to the start of the file
> @@ -2044,15 +2057,61 @@
> % Interpreting I/O error messages.
> %
> 
> -    % Construct an error code including the specified error message.
> +    % Construct an error value with the specified error message.
> +    % The error value will not have an associated system error.
>     %
> :- func make_io_error(string) = io.error.
> 
> -    % Look up the error message corresponding to a particular error code.
> +    % Return an error message for the error value.
>     %
> :- func error_message(io.error) = string.
> :- pred error_message(io.error::in, string::out) is det.

These will now need to take the I/O state. strerror and FormatMessage depend
on the current locale. That's going to breaking change, although I think we can
live with that, given that most calls to error_message occur in contexts where
the I/O state is available anyway.

Same goes for bunch of things in the implementation.
e.g system_error_errno_message, system_error_win32_error_message -- basically
anything that leads to call to strerror or FormatMessage.

> +    % get_system_error(Error, SystemError):
> +    %
> +    % Succeeds iff SystemError is a system error associated with Error.
> +    %
> +:- pred get_system_error(io.error::in, io.system_error::out) is semidet.
> +
> +    % As above, but only succeeds if the system error is an errno value.
> +    %
> +:- pred get_errno_error(io.error::in, io.system_error::out) is semidet.
> +
> +    % As above, but only succeeds if the system error is a Windows error code.
> +    % XXX ERROR: should this refer to "Win32" error codes instead?

Based on the blurb at the start of their API documentation, Microsoft would
_really_ like you to call it the Windows API, except for all of the places where
they still refer to it as the Win32 API ;-)  I think just Windows is fine.

> +:- pred get_windows_error(io.error::in, io.system_error::out) is semidet.
> +
> +    % As above, but only if the system error is a C# or Java exception object.
> +    % XXX ERROR: how to name this?
> +    %
> +:- pred get_error_exception_object(io.error::in, io.system_error::out)
> +    is semidet.

Given that all the other names have the form get_N_error, I suggest either

    get_exception_error

or:

    get_exception_object_error

...

> +    % get_system_error_name(Error, ErrorName):
> +    %
> +    % Succeeds if Error has an associated system error, otherwise fails.
> +    % On success, ErrorName is a name for that system error as follows.
> +    %
> +    % For C backends, a system error is usually an errno value. If the errno
> +    % value is recognised by the Mercury system, then ErrorName will be the
> +    % name for that errno value as defined in <errno.h>, e.g. "ENOENT".
> +    % Otherwise, ErrorName will be "errno: N" where N is a decimal number.
> +    %
> +    % For C backends on Windows, a system error may instead be a Windows system
> +    % error code. If the error code is recognised by the Mercury system, then
> +    % ErrorName will be the name for that error code in the Win32 API,

s/Win32/Windows/

> +    % e.g. "ERROR_FILE_NOT_FOUND". Otherwise, ErrorName will be
> +    % "System error: 0xN" where 0xN is a hexadecimal number.
> +    %
> +    % For the C# backend, ErrorName will be the the fully qualified class name

Double "the"

> +    % of an exception object, e.g. "System.IO.FileNotFoundException".
> +    %
> +    % For the Java backend, ErrorName will be the fully qualified class name
> +    % of an exception object, e.g. "java.io.FileNotFoundException".
> +    %
> +:- pred get_system_error_name(io.error::in, string::out) is semidet.

...

> -    % make_err_msg(Error, MessagePrefix, Message, !IO):
> -    % Message is an error message obtained by looking up the message for the
> -    % given errno value and prepending MessagePrefix.
> +    % Make an io.error from a message prefix and system error.
> +    % On Windows, the system error is assumed to be a errno value.
>     %
> -:- pred make_err_msg(system_error::in, string::in, string::out,
> -    io::di, io::uo) is det.
> +:- func make_io_error_from_system_error(string, io.system_error) = io.error.
> 
> -    % make_maybe_win32_err_msg(Error, MessagePrefix, Message, !IO):
> +    % Make an io.error from a message prefix and system error.
> +    % On Windows, the system error is assumed to be a Windows system error
> +    % code.

I would add: obtained by calling GetLastError().

> -    % Message is an error message obtained by looking up the error message
> -    % for Error and prepending MessagePrefix.
> -    % On Win32 systems, Error is obtained by calling GetLastError.
> -    % On other systems Error is obtained by reading errno.
> -    %
> -:- pred make_maybe_win32_err_msg(system_error::in, string::in, string::out,
> -    io::di, io::uo) is det.
> +:- func make_io_error_from_maybe_win32_error(string, io.system_error) =
> +    io.error.
>
>     % For use by bitmap.m, and other standard library modules
>     % that want to do I/O.
> @@ -2378,7 +2418,7 @@ io_state_compare(_, _, _) :-
> 
> open_input(FileName, Result, !IO) :-
>     do_open_text(FileName, "r", OpenCount, NewStream, Error, !IO),
> -    is_error(Error, "can't open input file: ", MaybeIOError, !IO),
> +    is_error(Error, "can't open input file: ", MaybeIOError),
>     (
>         MaybeIOError = yes(IOError),
>         Result = error(IOError)

...

> +%---------------------%
> +
> +    % XXX is FormatMessage thread-safe? Nothing suggests that it is not.

That information really ought to be easier to find than it apparently is :-(

> +    %
> +:- pred system_error_win32_error_message(io.system_error::in, string::out)
> +    is det.
> +
> +:- pragma foreign_proc("C",
> +    system_error_win32_error_message(ErrorCode::in, Name::out),
> +    [will_not_call_mercury, promise_pure, thread_safe, may_not_export_body],
> +"

...

> diff --git a/runtime/mercury_win32_error_name.c b/runtime/mercury_win32_error_name.c
> new file mode 100644
> index 000000000..72755462a
> --- /dev/null
> +++ b/runtime/mercury_win32_error_name.c

...

> diff --git a/runtime/mercury_win32_error_name.h b/runtime/mercury_win32_error_name.h
> new file mode 100644
> index 000000000..318dc2e8f

I would be inclined to name these mercury_windows_error_name.[ch].

Julien.