ClientWithResponses unmarshaling is not helpful for detecting errors in responses #2149
Description
Description
Similar to the situation described in #240, the ClientWithResponses generated code is not as helpful as it could be.
- We use
oapi-codegento quickly generate SDKs to interact with APIs that are documented with OpenAPI specs. - Using
ClientWithResponses.GetSomeOperation(), it is useful to have the code to parse the response body automatically generated. However, having to then check the response of each operation for the presence or absence ofJSON200,JSON401,JSON500, etc is more of a pain to deal with than checking the status code and doing the unmarshaling myself.
Currently, I have to write my own method for each operation response that is able to check the response status code, and return the appropriate unmarshaled response body or error. This is only works for APIs with very few operations and is already a pain to maintain.
As mentioned in #240, having access to an interface for responses makes it more maintainable to do the unmarhsaling and handling of error responses myself. However, I would also need access to the response header "Content-Type" in order to do so more easily.
Proposed behaviour
Client responses struct all implement the following interface:
type Response interface {
Status() string # Already done
StatusCode() int # Already done
GetBody() []byte # Done with config option 'client-response-bytes-function'
GetContentType() string # Proposition
}This would also help with other issues/open PR :
- Add an output option for generating a
defaultcase in response parsing function #1923 : makes it easy to check response status code, header and get the unmarshaled body or a default error - feat: opai-codegen#1912 add optional default format for numbers #1913 : enables using your own unmarshaler on a response struct
- feat(client): add abbility to set custom marshal/unmarshal func for client #2047 : enables writing your own unmarshaling code instead of relying on the one generated by oapi-codegen
Examples
Current situation
Code generated by oapi-codegen :
# Some structs a response can be unmarshaled into
type Foo struct {}
type SpecsError { Message string }
type ClientWithResponsesInterface interface {
GetTestWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetTestResponse, error)
}
type GetTestResponse struct {
Body []byte
HTTPResponse *http.Response
JSON200 *Foo
JSON4XX *SpecsError
}
// Status returns HTTPResponse.Status
func (r GetTestResponse) Status() string {
if r.HTTPResponse != nil {
return r.HTTPResponse.Status
}
return http.StatusText(0)
}
// StatusCode returns HTTPResponse.StatusCode
func (r GetTestResponse) StatusCode() int {
if r.HTTPResponse != nil {
return r.HTTPResponse.StatusCode
}
return 0
}
// Bytes is a convenience method to retrieve the raw bytes from the HTTP response
func (r GetTestResponse) Bytes() []byte {
return r.Body
}
func ParseGetTestResponse(rsp *http.Response) (*GetTestResponse, error) {
bodyBytes, err := io.ReadAll(rsp.Body)
defer func() { _ = rsp.Body.Close() }()
if err != nil {
return nil, err
}
response := &TestResponse{
Body: bodyBytes,
HTTPResponse: rsp,
}
switch {
case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode == 200:
var dest Foo
if err := json.Unmarshal(bodyBytes, &dest); err != nil {
return nil, err
}
response.JSON200 = &dest
case strings.Contains(rsp.Header.Get("Content-Type"), "json") && rsp.StatusCode/100 == 4:
var dest SpecsError
if err := json.Unmarshal(bodyBytes, &dest); err != nil {
return nil, err
}
response.JSON4XX= &dest
}
return response, nil
}Code I have to write :
resp, err := client.GetTestWithResponse(ctx)
if err != nil {
// log and handle error
}
if resp.JSON4XX != nil {
// turn response into a Go error
// log and handle error
}
// This can happen when the specifications are not complete and fail to include some errors on some operations
// ex: unauthorized errors are implied, but not explicitly added to the specs provided by a third-party
if resp.JSON200 == nil {
// check response status code
// turn response bytes into a Go error
// log and handle error
}
if resp.JSON200 != nil {
// response status code is successful, and unmarshaling did not fail
// still have to validate the unmarshaled JSON response fields afterwards to check pointers are not nil
}Helper I write for each operation to extend the code generated by oapi-codegen :
// Extending a generated model with its own logic for transforming it into a Go error
func (err SpecsError) AsError (error) {
return errors.New(err.Message)
}
// Extending a generated response wrapper for a specific operation, which has to be done for each operation
func (r GetTestResponse) ValidateResponse (*Foo, error) {
if r.StatusCode() == 200 && r.JSON200 != nil {
// add further validation as needed here
return r.JSON200, nil
}
if r.JSON4XX != nil {
return nil, r.JSON4XX.AsError()
}
return nil, fmt.Errorf("unexpected error: %q", r.Bytes())
}
// this then allows to write the following code
rawResp, err := client.GetTestWithResponse(ctx)
if err != nil {
// log and handle error
}
resp, err := rawResp.ValidateResponse()
if err != nil {
// log and handle error
}
// continue with successful response
With the proposed improvment in place
Code generated by oapi-codegen :
# Some structs a response can be unmarshaled into
type Foo struct {}
type SpecsError { Message string }
type ClientWithResponsesInterface interface {
GetTestWithResponse(ctx context.Context, reqEditors ...RequestEditorFn) (*GetTestResponse, error)
}
type GetTestResponse struct {
Body []byte
HTTPResponse *http.Response
JSON200 *Foo
JSON4XX *SpecsError
}
// Status returns HTTPResponse.Status
func (r GetTestResponse) Status() string {
if r.HTTPResponse != nil {
return r.HTTPResponse.Status
}
return http.StatusText(0)
}
// StatusCode returns HTTPResponse.StatusCode
func (r GetTestResponse) StatusCode() int {
if r.HTTPResponse != nil {
return r.HTTPResponse.StatusCode
}
return 0
}
// Bytes is a convenience method to retrieve the raw bytes from the HTTP response
func (r GetTestResponse) Bytes() []byte {
return r.Body
}
// A new convenience method to retrieve the response content type
func (r GetTestResponse) ContentType() string {
if r.HTTPResponse != nil {
return rsp.Header.Get("Content-Type")
}
return ""
}This example only serves to illustrate how much easier it becomes to handle responses in my scenario, where my API is able to return the same structured error for many operations. However, I believe that this is not an uncommon scenario, and that other possibilities also open up by having access to an interface for responses that grant you access to the response status code, its bytes and its content type. Unmarshaling logic can then extended by users without relying on oapi-codegen having to handle everything.
Code I can write :
type OapiCodegenResponse interface {
Status() string
StatusCode() int
GetBody() []byte
ContentType() string
}
func ParseJSONResponse[T any] (rsp OapiCodegenResponse, dest *T) (error) {
if rsp.StatusCode()/100 != 2 {
return ParseErrorResponse(rsp)
}
if rsp.ContentType() != "application/json" {
return errors.New("invalid content type received")
}
if err := json.Unmarshal(rsp.Bytes(), &dest); err != nil {
return err
}
return nil
}
func ParseErrorResponse(rsp OapiCodegenResponse) (error) {
if rsp.ContentType() != "application/json" {
return errors.New("unexpected error response")
}
var destError SpecsError
if err := json.Unmarshal(rsp.Bytes(), &destError); err != nil {
return err
}
return errors.New(destError.Message)
}
func ParsePDFResponse[T any] (rsp OapiCodegenResponse) ([]byte, error) {
if rsp.StatusCode()/100 != 2 {
return nil, ParseErrorResponse(rsp)
}
if rsp.ContentType() != "application/pdf" {
return nil, errors.New("invalid content type received")
}
if len(rsp.Bytes()) == 0 {
return nil, errors.New("empty response received")
}
return nil, resp.Bytes(), nil
}
// Those helpers can then be used this way :
rawResp, err := client.GetTestWithResponse(ctx)
if err != nil {
// log and handle error
}
var resp Foo
if err := ParseJSONResponse(rawResp, &resp); err != nil {
// log and handle error
}
// continue with successful response
// It is also easy to deal with responses where a successful response should not be unmarshaled, yet a failed one could :
rawFileResp, err := client.GetFileWithResponse(ctx)
if err != nil {
// log and handle error
}
fileBytes, err := ParsePDFResponse(rawResp)
if err != nil {
// log and handle error
}
// continue with file bytes available if the response was successful