Formatting of `OS.SystemError` messages

[jeremiah-corrado]

The SystemError class in the OS module currently produces error messages in the following format:

{Error-Class name}: {Error-Code description} ({message from the call-site})

In some cases, this implementation ends up producing a long or somewhat redundant string. For example:

NotADirectoryError: Not a directory (in moveDir(some/directory, not/a/directory.txt))

Should we rethink SystemErrors implementation of message()?

Would it be an improvement to remove the {Error-Code description} portion since these strings often align with the name of the error class?

[bradcray]

I definitely feel that there can be too much redundancy in errors like the above (and also generally don't like the parenthetical and the nested parens it results in, particularly since it seems like it could just be a part of the top-level "sentence".

I don't really know much about how these errors are implemented, but I wonder whether the {Error code description} could only be printed in the event that a more specific error has not been provided? And/or: Is the error class name actually useful to an end-user, or is that more like developer-level information? (e.g., Can I, as a user, do anything with that info? I suppose I could catch it now that I know its name? If so, that's a good argument to keep it).

[jeremiah-corrado]

I wonder whether the {Error code description} could only be printed in the event that a more specific error has not been provided?

Yeah I like that idea. I believe it would just be a small change to SystemErrors message() implementation. Specifically, replacing these lines:

chapel/modules/standard/OS.chpl

Lines 1079 to 1082 in 4b534d3

	if !details.isEmpty() then
	err_msg += " (" + details + ")";
	return err_msg;

with:

return if details.isEmpty() then err_msg else details;

I think keeping the error class name in the message makes sense for the reason you gave.

[mppf]

return if details.isEmpty() then err_msg else details;

I don't have all the code for this in front of me -- but wouldn't that lead to NotADirectoryError: Not a directory still? Which seems really redundant.

I think that for most of these error classes, which error code exactly it was is probably not that important in general, and we could probably drop {Error-Code description} in all cases.

Given all of that, I'd think we'd aim for something like this:

NotADirectoryError: in moveDir(some/directory, not/a/directory.txt)

(where perhaps we want to adjust something about in moveDir but I think this is the place to start).

[jeremiah-corrado]

wouldn't that lead to NotADirectoryError: Not a directory still?

That's true, but I think this seems like a reasonable way to handle the case where a details string is not provided to the constructor. I'm also open to leaving it as just NotADirectoryError: when details is left empty.

Of course, all of our internal uses of these error types should provide helpful details, but libraries that use them might not, so I like the idea of having a default behavior that can provide more information for some of the error codes.

[mppf]

I'm also open to leaving it as just NotADirectoryError: when details is left empty.

Or just NotADirectoryError (without the colon). IMO rewording the camel-case as non-camel case (which is what we have in practice here) is not helpful.