Provide an error message from Open api description #6953

koen-lee · 2025-09-25T12:07:41Z

See also #4349; this PR aims to change the developer experience from

ApiSdk.Models.ProblemDetails:Exception of type 'ApiSdk.Models.ProblemDetails' was thrown.
         at Microsoft.Kiota.Http.HttpClientLibrary.HttpClientRequestAdapter.ThrowIfFailedResponse....

to

ApiSdk.Models.ProblemDetails:403 Client certificate does not have required 'Write' role
         at Microsoft.Kiota.Http.HttpClientLibrary.HttpClientRequestAdapter.ThrowIfFailedResponse...

in case the OpenAPI document provides a description per status code.
The current behavior requires an extension for pretty error messages.

Before putting some tools to work to implement this for other languages, first I'd like to check whether this feature design fits the project.

It adds a constructor with a message to error classes and adds the description from the document to the error map:

            var errorMapping = new Dictionary<string, ParsableFactory<IParsable>>
            {
                { "400", (parseNode) => global::ApiSdk.Models.ValidationProblemDetails.CreateFromDiscriminatorValueWithMessage(parseNode, "400 Invalid hash format or hash mismatch") },
                { "401", (parseNode) => global::ApiSdk.Models.ProblemDetails.CreateFromDiscriminatorValueWithMessage(parseNode, "401 Missing or invalid client certificate") },
                { "403", (parseNode) => global::ApiSdk.Models.ProblemDetails.CreateFromDiscriminatorValueWithMessage(parseNode, "403 Client certificate does not have required 'Write' role") },
                { "409", (parseNode) => global::ApiSdk.Models.ProblemDetails.CreateFromDiscriminatorValueWithMessage(parseNode, "409 Upload for this hash already in progress, please retry") },
            };

The existing extension message logic is still intact, so this is used as fallback only.

to be passed to base, following C# conventions

Hopefully human-readable and helpful

…rwrite any previous constructor.

…flag is false

baywet

Thanks for the contribution!

src/Kiota.Builder/CodeDOM/CodeMethod.cs

baywet · 2025-10-01T12:44:53Z

src/Kiota.Builder/CodeDOM/CodeMethod.cs

+        errorMappings.TryAdd(errorCode, type);
+        if (!string.IsNullOrEmpty(description))


should the try add result be used in the condition here?

As I understand it, it is quite a corner case where consumer expectations are hard to guess - for this to matter, you'd need duplicate error code mappings where one has a description and the other hasn't.
But you have seen more OpenAPI specs than I so if you say this actually happens and the current behavior is unexpected, then the answer is yes :)

actually I didn't notice but we're duplicating a lot of code with the existing method.
Please:

add the description parameter as an optional parameter to the existing method

move the condition to the existing method

update the condition to consider the try add, so we don't end up in a weird state where we have a description for an error type that doesn't exist

remove this new method

Update the GetErrorDescriptionMethod to always check whether a matching mapping is present before returning anything (same principle, safety first)

src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs

src/Kiota.Builder/Refiners/CSharpRefiner.cs

src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs

static lambdas, simplification. Co-authored-by: Vincent Biret <[email protected]>

koen-lee · 2025-10-02T12:06:29Z

@baywet thanks for the review; I think all your points have been addressed.
We could also add a CodeMethodKind ConstructorWithMessage but I see little value in that.

Is it OK to proceed with other languages in the same manner or is there still room for improvement?

baywet · 2025-10-03T12:44:15Z

tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs

+        // Then
+        var messageConstructor = errorClass.Methods
+            .FirstOrDefault(m => m.IsOfKind(CodeMethodKind.Constructor) &&
+                                m.Parameters.Any(p => p.Type.Name.Equals("string", StringComparison.OrdinalIgnoreCase) && p.Name.Equals("message", StringComparison.OrdinalIgnoreCase)));


this should test on the new kind that was introduced (multiple instances)

baywet · 2025-10-03T12:44:26Z

src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs

        }
+        // For error classes with message constructor, pass the message to base constructor
+        else if (isConstructor && parentClass.IsErrorDefinition &&
+                 currentMethod.Parameters.Any(p => p.IsOfKind(CodeParameterKind.ErrorMessage)))


Suggested change

currentMethod.Parameters.Any(p => p.IsOfKind(CodeParameterKind.ErrorMessage)))

currentMethod.Parameters.Any(static p => p.IsOfKind(CodeParameterKind.ErrorMessage)))

There's probably a static analyser trick I'm missing here that would warn contributors about lambdas that can be static?

baywet · 2025-10-03T12:44:43Z

src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs

            WriteFactoryMethodBodyForIntersectionModel(codeElement, parentClass, parseNodeParameter, writer);
+        else if (codeElement.IsOfKind(CodeMethodKind.FactoryWithErrorMessage))
+        {
+            var messageParam = codeElement.Parameters.FirstOrDefault(p => p.IsOfKind(CodeParameterKind.ErrorMessage)) ?? throw new InvalidOperationException($"FactoryWithErrorMessage should have a message parameter");


Suggested change

var messageParam = codeElement.Parameters.FirstOrDefault(p => p.IsOfKind(CodeParameterKind.ErrorMessage)) ?? throw new InvalidOperationException($"FactoryWithErrorMessage should have a message parameter");

var messageParam = codeElement.Parameters.FirstOrDefault(static p => p.IsOfKind(CodeParameterKind.ErrorMessage)) ?? throw new InvalidOperationException($"FactoryWithErrorMessage should have a message parameter");

baywet · 2025-10-03T12:52:00Z

src/Kiota.Builder/CodeDOM/CodeMethod.cs

+        errorMappings.TryAdd(errorCode, type);
+        if (!string.IsNullOrEmpty(description))


actually I didn't notice but we're duplicating a lot of code with the existing method.
Please:

add the description parameter as an optional parameter to the existing method

move the condition to the existing method

update the condition to consider the try add, so we don't end up in a weird state where we have a description for an error type that doesn't exist

remove this new method

Update the GetErrorDescriptionMethod to always check whether a matching mapping is present before returning anything (same principle, safety first)

baywet · 2025-10-03T12:53:27Z

src/Kiota.Builder/Refiners/CSharpRefiner.cs

+            }
+
+            // Add message constructor if not already present
+            if (!codeClass.Methods.Any(static x => x.IsOfKind(CodeMethodKind.Constructor) && x.Parameters.Any(static p => "string".Equals(p.Type.Name, StringComparison.OrdinalIgnoreCase))))


should filter on the new kind instead of the name

koen-lee · 2025-10-03T14:18:44Z

src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs

+                if (errorMapping.Value.AllTypes.FirstOrDefault()?.TypeDefinition is CodeClass { IsErrorDefinition: true } errorClass)
+                {
+                    var errorDescription = codeElement.GetErrorDescription(errorMapping.Key);
+                    var statusCodeAndDescription = !string.IsNullOrEmpty(errorDescription)


@baywet I'm having second thoughts on this - I think we shouldn't include the status code key here because it may be confusing in the 4XX/5XX case.
So, just the description when provided and the old behavior when not.
The actual specific response status code has value though; #6951 would help to provide that without conflicting.

to avoid confusion with 4xx 5xx generic codes

koen-lee added 6 commits September 24, 2025 13:25

Pass the description along with the response code

bfc848a

Give an error class a constructor that takes a message

1c3d3e8

to be passed to base, following C# conventions

Have the error mapping include the descriptions

b4c5f47

Hopefully human-readable and helpful

cleanup / deduplicate

9bd10a8

Don't add without parameters and add parameters later, as it will ove…

3d40934

…rwrite any previous constructor.

THough all error definitions inherit from ApiException, the inherits …

2131100

…flag is false

koen-lee requested a review from a team as a code owner September 25, 2025 12:07

baywet requested changes Oct 1, 2025

View reviewed changes

koen-lee and others added 2 commits October 2, 2025 11:46

Partially apply suggestions from code review

e287fd2

static lambdas, simplification. Co-authored-by: Vincent Biret <[email protected]>

Prefer enums over magic strings

19491e5

baywet reviewed Oct 3, 2025

View reviewed changes

baywet mentioned this pull request Oct 3, 2025

Override ApiException.Message getter to include ResponseStatusCode #6951

Open

Simplify, address more review comments

159e96a

koen-lee commented Oct 3, 2025

View reviewed changes

Don't include the template status code

5856ca0

to avoid confusion with 4xx 5xx generic codes

		errorMappings.TryAdd(errorCode, type);
		if (!string.IsNullOrEmpty(description))

	currentMethod.Parameters.Any(p => p.IsOfKind(CodeParameterKind.ErrorMessage)))
	currentMethod.Parameters.Any(static p => p.IsOfKind(CodeParameterKind.ErrorMessage)))

	var messageParam = codeElement.Parameters.FirstOrDefault(p => p.IsOfKind(CodeParameterKind.ErrorMessage)) ?? throw new InvalidOperationException($"FactoryWithErrorMessage should have a message parameter");
	var messageParam = codeElement.Parameters.FirstOrDefault(static p => p.IsOfKind(CodeParameterKind.ErrorMessage)) ?? throw new InvalidOperationException($"FactoryWithErrorMessage should have a message parameter");

Provide an error message from Open api description #6953

Are you sure you want to change the base?

Provide an error message from Open api description #6953

Uh oh!

Conversation

koen-lee commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

baywet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koen-lee commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

koen-lee commented Sep 25, 2025 •

edited

Loading

koen-lee commented Oct 2, 2025 •

edited

Loading