os: add error_posix() and error_win32() for explicit platform error handling and default behavior

[syrmel]

In the os module, in some places, errors are returned with the POSIX error message and C.errno
return error_with_code(posix_get_error_msg(C.errno), C.errno)
In other places, errors are swallowed and the error code is lost (in vfopen())
return error('failed to open file "${path}"')

It is often desirable to override the default error message to provide additional information (like ${path} in the example above).

Therefore, this PR proposes an explicit function error_posix() with default behavior of
error_with_code(posix_get_error_msg(C.errno), C.errno)
and the option to override the error message (msg) or error code (code) when desired.
Example: return error_posix(msg: 'failed to open file "${path}"')
In this case, the returned IError would still contain C.errno as its error code.

error_win32 works analogous using int(C.GetLastError()) and get_error_msg() to get Win32 API errors.

Using error_posix and error_win32 enables consistent default behavior, ensures the API error codes do not get lost, and makes it easier for developers to see that the error handling matches the used API.

[JalonSolov]

Hmm... I applaud the new routines, but I'm not a fan of making platform specific functions, as that now forces extra comptime code or other nonsense if you want the code to work on all platforms.

I think I'd rather see error_with_code changed so that it returned the platform default msg+code if you don't give any parameters, or you can override them by supplying your own.

Param struct to the rescue, here.

[spytheman]

Excellent work.

[spytheman]

I think I'd rather see error_with_code changed so that it returned the platform default msg+code if you don't give any parameters, or you can override them by supplying your own.

I am not sure I understand your idea. Note that on windows, some APIs do set C.errno, while some others rely on the caller to call C.GetLastError(). How will you tell error_with_code which is the case?

I think the solution in this PR, solves it in a much more straightforward way, by introducing a new error, and 2 constructor functions for it, then all callsites can decide for themselves what do they want, and it is easy to check for erroneous usages.

[spytheman]

This specific function could potentially move to vlib/os/os_windows.c.v so that using it is a comptime error in the future, although that may break bootstrapping 🤔 now.

We could also use $compile_error('...') here instead of the runtime panic, once that works reliably.

[syrmel]

I think I'd rather see error_with_code changed so that it returned the platform default msg+code if you don't give any parameters, or you can override them by supplying your own.

I am not sure I understand your idea. Note that on windows, some APIs do set C.errno, while some others rely on the caller to call C.GetLastError(). How will you tell error_with_code which is the case?

The way I understand @JalonSolov's idea is something like this:

const use_c_errno = 0x7EFEFEFE

@[params]
struct ApiError {
	msg  string
	code int = os.use_c_errno
}

// Return an API error:
// Code defaults to last error (from C.errno)
// Message defaults to the platform default error message corresponding
// to the error code
@[inline]
pub fn error_from_api(e ApiError) IError {
	code := if e.code == os.use_c_errno { C.errno } else { e.code }
	// get_error_msg returns the platform-specific error message
	message := if e.msg == '' { get_error_msg(code) } else { e.msg }
	return error_with_code(message, code)
}

[JalonSolov]

Sort of. I was more suggesting that error_with_code be changed to essentially what your error_from_api function does. error_with_code already exists, and that seems like what it should have been doing all along.

[syrmel]

Sort of. I was more suggesting that error_with_code be changed to essentially what your error_from_api function does. error_with_code already exists, and that seems like what it should have been doing all along.

I see. I did not change the builtin error_with_code to a params struct. I assume it is in use quite a bit already and would force people to have to use it with named parameters now when calling it.

[JalonSolov]

There are about 100 calls to that function in all the main V source, so that's not too terrible.

Of the 18 modules I have installed (including all the main V ones, including vsl, vtl, ui, etc.), there is exactly 1 call in the markdown module.

So, simple enough to update.

[spytheman]

No, error_with_code is simple, and should not be changed. Another function is ok.