From 308a56b40e78ac4ebf25eaacf0d5dac248f4f80e Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:59:28 +0100 Subject: [PATCH] fix: a refusal reaches the practitioner in the API's own words kiota deserializes an error body into the properties the document declared and never copies any of them onto the embedded ApiError, whose Error() then answers one constant however much the API said. Every refusal a generated provider reported read "error status code received from the API", which names no field, no value and no condition. The status alone does not separate them. Against one pilot a single 400 stood for a name collision, a missing field, an unreachable URL, an absent discriminator, an empty relation and an opaque validation failure; the message separates five of the six. The kiota extractor now reads what the API sent: the problem document's summary and explanation joined when both are present and distinct, then the single property that carried a sentence, then the listed complaints among the properties the document did not declare. The vocabulary is the audit's own refusal reader, so both stages read a refusal the same way rather than inventing a second spelling for it. This brings the dialect level with the other one: the openapi-generator extractor already answered with the raw response body. Co-Authored-By: Claude Opus 5 (1M context) --- internal/emit/render_test.go | 43 ++++++ .../common/errors/extract_kiota.go.tmpl | 129 +++++++++++++++++- 2 files changed, 169 insertions(+), 3 deletions(-) diff --git a/internal/emit/render_test.go b/internal/emit/render_test.go index 13b0dd8..b7ce46b 100644 --- a/internal/emit/render_test.go +++ b/internal/emit/render_test.go @@ -439,3 +439,46 @@ func TestUnit_RenderProviderCore_NonGoFilesAreNotHeldToGofmt(t *testing.T) { t.Fatalf("got %q, want %q", got, want) } } + +// TestUnit_RenderProviderCore_BothDialectsCarryWhatTheAPISaid proves neither +// generated extractor reports a refusal as the SDK's own constant when the +// response body carried the API's words. +func TestUnit_RenderProviderCore_BothDialectsCarryWhatTheAPISaid(t *testing.T) { + for backend, want := range map[string][]string{ + // kiota deserializes the body into the properties the document + // declared and leaves the embedded ApiError's message unset, so the + // getters are the only place the API's words survive. + config.BackendKiota: { + "kiotaSilence", + "GetDetail() *string", + "GetTitle() *string", + "GetAdditionalData() map[string]any", + "validationErrors", + }, + // openapi-generator hands the raw body over, so reading it is enough. + config.BackendOpenAPIGenerator: {"apiErr.Body()"}, + } { + pc, err := FromConfig(testConfig(backend, config.AuthBearerToken), "") + if err != nil { + t.Fatalf("FromConfig(%s): %v", backend, err) + } + files, err := RenderProviderCore(pc) + if err != nil { + t.Fatalf("RenderProviderCore(%s): %v", backend, err) + } + var extractor string + for _, f := range files { + if strings.HasPrefix(f.Path, "internal/services/common/errors/extract_") { + extractor = string(f.Content) + } + } + if extractor == "" { + t.Fatalf("%s rendered no extractor", backend) + } + for _, fragment := range want { + if !strings.Contains(extractor, fragment) { + t.Errorf("%s extractor does not read %q", backend, fragment) + } + } + } +} diff --git a/internal/templates/provider-core/internal/services/common/errors/extract_kiota.go.tmpl b/internal/templates/provider-core/internal/services/common/errors/extract_kiota.go.tmpl index 262aacb..31dc4a4 100644 --- a/internal/templates/provider-core/internal/services/common/errors/extract_kiota.go.tmpl +++ b/internal/templates/provider-core/internal/services/common/errors/extract_kiota.go.tmpl @@ -4,6 +4,8 @@ package errors import ( stderrors "errors" + "fmt" + "strings" ) // kiotaAPIError is what every kiota-generated error type satisfies by @@ -14,6 +16,26 @@ type kiotaAPIError interface { GetStatusCode() int } +// kiotaSilence is what ApiError.Error() answers when nothing set its Message. +// +// kiota deserializes an error body into the properties the document declared +// and never copies any of them onto the embedded parent, so every generated +// error type answers this one constant however much the API said. Treated as +// silence rather than as a message: it names no field, no value and no +// condition, and a practitioner reading it learns only that something failed. +const kiotaSilence = "error status code received from the API" + +// The error-body accessors a generated type carries. kiota names a getter +// after the property its own schema declared, so a type has the spellings +// that document used and no others, and each is asserted separately. +type ( + kiotaDetailed interface{ GetDetail() *string } + kiotaTitled interface{ GetTitle() *string } + kiotaMessaged interface{ GetMessage() *string } + kiotaErrored interface{ GetError() *string } + kiotaUndeclared interface{ GetAdditionalData() map[string]any } +) + // kiotaExtractor reads the kiota SDK's error shape. type kiotaExtractor struct{} @@ -23,10 +45,111 @@ var wire extractor = kiotaExtractor{} // extract answers for any error in the chain that carries a kiota API // error; everything else is not an API error. +// +// What the API said outranks what the SDK said: the SDK's own Error() is a +// constant unless something set it, and the words that name the refused field +// are on the generated type's getters. func (kiotaExtractor) extract(err error) (Info, bool) { var apiErr kiotaAPIError - if stderrors.As(err, &apiErr) { - return Info{Status: apiErr.GetStatusCode(), Message: apiErr.Error()}, true + if !stderrors.As(err, &apiErr) { + return Info{}, false + } + info := Info{Status: apiErr.GetStatusCode(), Message: apiErr.Error()} + if said := kiotaSaid(apiErr); said != "" { + info.Message = said + } + return info, true +} + +// kiotaSaid answers what the API's error body carried, and empty when it +// carried nothing this SDK kept. +// +// A problem document sends a summary and a specific explanation — "Request +// validation failed" and "Required request body is missing mandatory field" — +// and only the pair is worth acting on, so both are joined when both are +// present and distinct. Everything else falls back to the single property +// that carried a sentence. +func kiotaSaid(err error) string { + var said []string + if v, ok := err.(kiotaTitled); ok { + said = appendText(said, v.GetTitle()) + } + if v, ok := err.(kiotaDetailed); ok { + said = appendText(said, v.GetDetail()) + } + if len(said) == 0 { + if v, ok := err.(kiotaMessaged); ok { + said = appendText(said, v.GetMessage()) + } + if v, ok := err.(kiotaErrored); ok { + said = appendText(said, v.GetError()) + } + } + if len(said) == 0 { + if v, ok := err.(kiotaUndeclared); ok { + return undeclaredText(v.GetAdditionalData()) + } + } + return strings.Join(said, ": ") +} + +// appendText adds one non-empty, not-already-present string. +// +// A body that repeats its summary as its explanation is common, and saying it +// twice reads as two problems. +func appendText(said []string, text *string) []string { + if text == nil || strings.TrimSpace(*text) == "" { + return said + } + trimmed := strings.TrimSpace(*text) + for _, existing := range said { + if existing == trimmed { + return said + } + } + return append(said, trimmed) +} + +// undeclaredText reads a message out of the properties the document did not +// declare, which kiota keeps rather than discards. +// +// The listed complaints come first: an envelope that carries both names the +// field it rejected in the list and only summarises it in the sentence, and a +// summary names nothing to act on. +func undeclaredText(extra map[string]any) string { + for _, key := range []string{"errors", "messages", "details", "errorMessages", "validationErrors"} { + if listed := listedText(extra[key]); listed != "" { + return listed + } + } + for _, key := range []string{"detail", "message", "error_description", "errorMessage", "error", "title"} { + if text, ok := extra[key].(string); ok && strings.TrimSpace(text) != "" { + return strings.TrimSpace(text) + } + } + return "" +} + +// listedText renders the first entry of a list of complaints, spelling an +// object entry back as ": ". +func listedText(value any) string { + entries, ok := value.([]any) + if !ok || len(entries) == 0 { + return "" + } + switch first := entries[0].(type) { + case string: + return strings.TrimSpace(first) + case map[string]any: + field, _ := first["field"].(string) + for _, key := range []string{"message", "detail", "error", "reason"} { + if text, ok := first[key].(string); ok && strings.TrimSpace(text) != "" { + if field != "" { + return fmt.Sprintf("%s: %s", field, strings.TrimSpace(text)) + } + return strings.TrimSpace(text) + } + } } - return Info{}, false + return "" }