openapi-generator: [BUG][GO][Client] Returned err is never nil

Bug Report Checklist

Have you provided a full/minimal spec to reproduce the issue?
Have you validated the input using an OpenAPI validator (example)?
Have you tested with the latest master to confirm the issue still exists?
Have you searched for related issues/PRs?
What’s the actual output vs expected output?

Description

When calling any generated Execute() function, a non-nil error is always returned. Instead of using the usual Go idiom:

result, _, err := r.Execute()
if err != nil {
	/* deal with error */
}

I need to check the actual error message in order to detect errors:

result, _, err := r.Execute()
if err != nil && err.Error() != "" {
	/* deal with error */
}

openapi-generator version

c3220848 (current main branch)

Related issues/PRs

I believe this was introduced by #8137. Maybe it is intended behaviour. Care to weigh in here, @code-lucidal58?

Suggest a fix

Successful execution of *Execute() should not return a non-nil error.

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 5
Comments: 18 (12 by maintainers)

Most upvoted comments

@code-lucidal58, the semantic of golang error will be broken. Some part of the community uses codegen to generate wrappers around the API clients for writing logs, metrics, traces etc. They highly depend on the error type. E.g. gowrap template for opentracing.

Let’s look closer to the idea of returning error type: If you care about readability – we can use a helper function to extract necessary data using type assertion. If you care about performance – I’m sure that single type assertion Is not the main performance issue in call processing.

It would be great to hear thoughts of the other community members.

cheshir on Jan 7, 2021

I’m in favor of reverting to the previous behaviour to keep the semantics of golang error.

I think the helper function is a good way to improve readability and avoid too many code duplications when dealing with errors. This is what is done in our terraform provider for instance, which uses a generated go client:

Cheers

zippolyte on Jan 8, 2021

since this is not Golang error.

Excuse me, but what does it mean? Which one this error is then? It implements standard lib error interface, so one would expect standard behaviour, including nil comparison. Since this is literally the breaking change, may it at least be done as in previous versions - without local var executionError and returning nil on successful path?

alholm on Jan 5, 2021

pointer to a struct (or any variable) will never be nil. https://play.golang.org/p/53vSwOYcCV0 even if it’s an empty struct. The return type was earlier error. GenericOpenAPIError was typecasted to error. This lead to the loss of values stored in body and model. error supports only one method Error(), neitherBody() nor Model().

code-lucidal58 on Dec 24, 2020

What if we return a *GenericOpenAPIError pointer (allowing it to be nil)? That way users will be able to use the usual err != nil idiom. It may even be declared as error in the function header since it satisfies the error interface. Will that interfere with the response body?

thiagoarrais on Dec 23, 2020

The 5.0.0-beta3 version has native for go developers behaviour – methods return error type. Not the custom structure. GenericOpenAPIError.Error() method confuses because it uses the same signature as an error interface. So I and many other developers think that we can use it in the same way but its not true.

It would be awesome if you will alow users to configure generator to select desired behaviour: generate methods with custom error or with error interface that covered custom error type.

cheshir on Jan 5, 2021