[API Proposal]: Expose method to get system message for system error code

[danmoseley]

Background and motivation

This optionally (and typically) retrieves the last system error (with Marshal.GetLastPInvokeError()) and the matching system error string (internally using strerror or FormatMessage) for you:

throw new Win32Exception();

(Separate discussion of the unfortunate naming we have inherited for Win32Exception is here.)
(Also note that Marshal.GetLastPInvokeError() == Marshal.GetLastWin32Error() and is distinct to Marshal.GetLastSystemError() only in that the latter is oblivious to calls to Marshal.SetLastPInvokeError() == Marshal.SetLastWin32Error)

This is convenient if that's exactly what you want, but has these limitations

1. It's not discoverable, you just have to know.

2. It's easy to accidentally reset the error, eg.,

// some pinvoke
// some other code potentially resetting the last error by calling into the system directly or indirectly
throw new Win32Exception(); // uses the last error code, when perhaps you expected a generic message

correct way:

// some pinvoke
int errorCode = Marshal.GetLastWin32Error();
// some other code
throw new Win32Exception(errorCode);

3. Sometimes you just want the message, you don't want an exception. For example, you want to write to the system log, or do some tracing, like this), In this case getting the message is clumsy:

string errorMessage = new Win32Exception().Message;
WriteLogEntry(SR.Format(SR.StartFailed, errorMessage), EventLogEntryType.Error);

or if you want it from the error code:

string errorMessage = new Win32Exception(errorCode).Message;
WriteLogEntry(SR.Format(SR.StartFailed, errorMessage), EventLogEntryType.Error);

4. If you want to throw a partially custom message, you have to be even more clumsy, and it's easy to accidentally overwrite the last error.

This is not robust because some string formatting machinery runs and potentially resets the last error before Win32Exception internally calls Marshal.GetLastPInvokeError():

throw new Win32Exception($"Could not open SCM. {new Win32Exception().Message}");

This is not robust either because Win32Exception still calls Marshal.GetLastPInvokeError() to set its NativeErrorCode. #67827 is an example where this was wrong.

string errorMessage = new Win32Exception().Message;
throw new Win32Exception($"Could not delete service. {errorMessage}");

One must write this to be robust (example in our code)

int errorCode = Marshal.GetLastWin32Error();
string errorMessage = new Win32Exception(errorCode).Message;
throw new Win32Exception(errorCode, $"Could not delete service. {errorMessage}");

It's unfortunate that Win32Exception calls Marshal.GetLastPInvokeError() at all but that can't change now. We can at least help address 1, 3 and 4.

API Proposal

Proposed API implementation - #70685

#70685 (comment) has commentary on proposed API UX.

namespace System.Runtime.InteropServices
{
    public class Marshal
    {
        public static int GetLastPInvokeError(); // existing
+       public static string GetPInvokeErrorMessage(int error); // Win32Exception parameter name is `error`, although its property is `NativeErrorCode`
    }
}

API Usage

int errorCode = Marshal.GetLastPInvokeError(); // error is captured here
string errorMessage = Marshal.GetPInvokeErrorMessage(errorCode);
throw new Win32Exception(errorCode, $"Could not delete service. {errorMessage}");

or

int errorCode = Marshal.GetLastPInvokeError(); // error is captured here
throw new Win32Exception(errorCode, $"Could not delete service. {Marshal.GetPInvokeErrorMessage(errorCode)}");

Alternative Designs

It would be nice to add overloads to Win32Exception, that eg., take a message to prepend. However, that may not be a common enough requirement, and would be ambiguous with existing string overload. It would probably have to be a static factory method. You'd still need the Marshal method, in case you don't want the exception object.

Another possibility is to return message and code together as a tuple, eg

namespace System.Runtime.InteropServices
{
    public class Marshal
    {
        public static (int error, string message) GetLastPInvokeErrorMessage();
    }
}

Risks

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

This optionally (and typically) retrieves the last system error (with Marshal.GetLastPinvokeError()) and the matching system error string (internally using strerror or FormatMessage) for you:

throw new Win32Exception();

(Separate discussion of the unfortunate naming we have inherited for Win32Exception is here.)
(Also note that Marshal.GetLastPinvokeError() == Marshal.GetLastWin32Error() and is distinct to Marshal.GetLastSystemError() only in that the latter is oblivious to calls to Marshal.SetLastPInvokeError() == Marshal.SetLastWin32Error)

This is convenient if that's exactly what you want, but has these limitations

1. It's not discoverable, you just have to know.

2. It's easy to accidentally reset the error, eg.,

// some pinvoke
// some other code potentially resetting the last error by calling into the system directly or indirectly
throw new Win32Exception(); // uses the last error code, when perhaps you expected a generic message

correct way:

// some pinvoke
int errorCode = Marshal.GetLastWin32Error();
// some other code
throw new Win32Exception(errorCode);

3. Sometimes you just want the message, you don't want an exception. For example, you want to write to the system log, or do some tracing, like this), In this case getting the message is clumsy:

string errorMessage = new Win32Exception().Message;

or if you want it from the error code:

string errorMessage = new Win32Exception(errorCode).Message;

4. If you want to throw a partially custom message, you have to be even more clumsy, and it's easy to accidentally overwrite the last error.

This is not robust because some string formatting machinery runs and potentially resets the last error before Win32Exception internally calls Marshal.GetLastPinvokeError():

throw new Win32Exception($"Could not open SCM. {new Win32Exception().Message}");

This is not robust either because Win32Exception still calls Marshal.GetLastPinvokeError() to set its NativeErrorCode. #67827 is an example where this was wrong.

string errorMessage = new Win32Exception().Message;
throw new Win32Exception($"Could not delete service '{ServiceName}'. {errorMessage}");

One must write this to be robust (example in our code)

int errorCode = Marshal.GetLastWin32Error();
string errorMessage = new Win32Exception(errorCode).Message;
throw new Win32Exception(errorCode, $"Could not delete service '{ServiceName}'. {errorMessage}");

It's unfortunate that Win32Exception calls Marshal.GetLastPinvokeError() at all but that can't change now. We can at least help address 1, 3 and 4.

API Proposal

namespace System.Runtime.InteropServices
{
    public class Marshal
    {
        public static int GetLastPInvokeError(); // existing
        public static string GetMessageforPinvokeError(int error); // Win32Exception parameter name is `error`, although its property is `NativeErrorCode`
    }
}

API Usage

int errorCode = Marshal.GetLastPinvokeError(); // error is captured here
string errorMessage = Marshal.GetMessageforPinvokeError(errorCode);
throw new Win32Exception(errorCode, $"Could not delete service '{ServiceName}'. {errorMessage}");

or

int errorCode = Marshal.GetLastPinvokeError(); // error is captured here
throw new Win32Exception(errorCode, $"Could not delete service '{ServiceName}'. {Marshal.GetMessageforPinvokeError(errorCode)}");

Alternative Designs

It would be nice to add overloads to Win32Exception, that eg., take a message to prepend. However, that may not be a common enough requirement, and would be ambiguous with existing string overload. It would probably have to be a static factory method. You'd still need the Marshal method, in case you don't want the exception object.

Risks

No response

Author:	danmoseley
Assignees:	-
Labels:	api-suggestion, area-System.Runtime.InteropServices, untriaged
Milestone:	-

[jkotas]

Nit: GetMessageforPinvokeError -> GetMessageforPInvokeError

[danmoseley]

fixed.

[stephentoub]

GetPInvokeErrorMessage?

[danmoseley]

That's an improvement. Updated.

[danmoseley]

@AaronRobinsonMSFT if you're supportive, shall we mark ready for review?

[danmoseley]

Setting milestone as I'd like to get this in this release.

[AaronRobinsonMSFT]

@danmoseley Do you want to present this to the API review board? I've put out a PR as proposed above.

[danmoseley]

@stephentoub suggested it wasn't necessary for me to be there, although I can be if necessary if someone lets me know which one it will be at. If you attend perhaps you could speak to it?

[AaronRobinsonMSFT]

If you attend perhaps you could speak to it?

Sure.

[terrajobst]

Video

• Let's also add GetLastPInvokeErrorMessage()

namespace System.Runtime.InteropServices;

public partial class Marshal
{
    // Existing
    // public static int GetLastPInvokeError();
    public static string GetLastPInvokeErrorMessage();
    public static string GetPInvokeErrorMessage(int error);
}