Repository navigation
Conversation
…es in ApiException When RestSharp does not deserialize the body itself (for example a text/plain response), ErrorException stays null and the oneOf/anyOf FromJson call is the first to fail. Its InvalidDataException reached the caller without status code, body or headers, although throwOnAnyError promises an ApiException. Wrap it like the ErrorException case, with the parse error as the inner exception, in both restsharp ApiClient templates. Fixes OpenAPITools#25218
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
@ybai08 thanks for the PR can you please review the feedback from cubic-dev-ai? |
…es to ExceptionFactory The oneOf/anyOf FromJson wrap threw ApiException for every status code, so a 4xx/5xx body matching none of the schemas never reached ExceptionFactory and a custom factory was ignored for those operations. Wrap only status codes below 400, like the ErrorException check above it, and let 4xx/5xx fall through to ExceptionFactory as other operations do.
|
@wing328 thanks, the cubic finding is valid. Fixed in 3db0ecf. Under The What I ran:
Not run: the full |
Fixes #25218.
With
throwOnAnyError=true, the restsharpApiClientturns a recordedErrorExceptioninto anApiException(#25217). TheoneOf/anyOfbranch after that check callsFromJsonitself, and when that throws, thecatchrethrew the raw inner exception. So a response that RestSharp did not try to deserialize, but that matches none of the schemas, reached the caller as a bareInvalidDataExceptionwith no status code, body or headers.Under
throwOnAnyError, thecatchnow throwsApiException(status, "Error calling <resource>: <message>", content, headers, inner), the same shape as theErrorExceptionguard just above it, with theFromJsonerror kept asInnerException. WithoutthrowOnAnyErrorthe generated code is unchanged. The change is in both restsharp templates,ApiClient.mustacheand theuseIntForTimeoutfallbackApiClient.v790.mustache.What I measured, on a generated
net10.0restsharp client for a spec whoseGET /petreturnsoneOf: [Cat, Dog](bothrequired+additionalProperties: false), against a localHttpListenerreturning 200:text/plain, bodyOKSystem.IO.InvalidDataException("The JSON string OK cannot be deserialized into any schema defined.")ApiExceptionErrorCode=200, ErrorContent=OK, headers kept, InnerExceptionInvalidDataExceptionapplication/json, body{"swims":true}ApiException(200)(innerApiException)text/html, an HTML pageApiException(200)(innerInvalidOperationException)So for a JSON body the #25217 guard already fires, because RestSharp's own deserialization fails first and sets
ErrorException. The gap is a response RestSharp leaves alone, such astext/plain. Same results withuseIntForTimeout=true(the v790 template).A 4xx/5xx response whose body matches none of the schemas is left to
ExceptionFactory, asthrowOnAnyErroralready does for other operations: the wrap applies only below 400 (since 3db0ecf, after review), so a custom factory is not bypassed. Before this PR such a response got the rawInvalidDataException.Tests
CSharpClientCodegenTest.testThrowOnAnyErrorWrapsOneOfDeserializationFailure: generates a restsharp client for each combination ofthrowOnAnyErroranduseIntForTimeout, and checksApiClient.cshas the wrappingthrow new ApiException(...)only whenthrowOnAnyErroris on (and the old rethrow otherwise). Fails on master templates (does not contain line [throw new ApiException(...)]). It also fails when onlyApiClient.mustacheis fixed, which covers the v790 template../mvnw test -pl modules/openapi-generator -am -Dtest=CSharpClientCodegenTest: 27 tests, 0 failures../mvnw test -pl modules/openapi-generator -am(JDK 21, macOS arm64): 5574 tests, 0 failures, 0 errors, 11 skipped../bin/generate-samples.sh bin/configs/csharp-restsharp-net4.7-multipleFrameworks.yaml(the only config withthrowOnAnyError). One file changes:Client/ApiClient.cs, +2/-1. Built the regenerated library withdotnet build -f netstandard2.1(.NET SDK 10.0.401, macOS arm64): succeeded. Itsnet47/net7.0test project wasn't run here (no Mono, no .NET 7 runtime), andThrowOnAnyErrorTestsdoesn't cover this path because the petstore spec has nooneOfmodel.PR checklist
./mvnw clean package,./bin/generate-samples.sh ./bin/configs/*.yamland./bin/utils/export_docs_generators.sh. Instead:./mvnw package -pl modules/openapi-generator-cli -am -DskipTests, themodules/openapi-generatortests, and./bin/generate-samples.sh bin/configs/csharp-restsharp-net4.7-multipleFrameworks.yaml. The change sits inside{{#throwOnAnyError}}/{{^throwOnAnyError}}, and the other C# configs don't set that option, so their output is unchanged. No option or doc changed.Summary by cubic
Fixes #25218 so
throwOnAnyErrorwrapsoneOf/anyOfdeserialization failures in anApiExceptionfor responses below 400, while leaving 4xx/5xx toExceptionFactoryas before.InvalidDataException(no status code, body, or headers) surfaced when RestSharp did not deserialize the body (e.g.,text/plain).ErrorExceptionguard, keeping the parse error asInnerException, in bothApiClient.mustacheand theuseIntForTimeoutfallbackApiClient.v790.mustache.ExceptionFactory, so custom factories aren't bypassed.throwOnAnyErroris off.Written for commit 3db0ecf. Summary will update on new commits.