StatusCode check in api_grafeas_v1_beta1.go

[lynnsh]

Problem

Nested if statement that checks for StatusCode == 200 is always ignored:

if localVarHttpResponse.StatusCode < 300 {
	// If we succeed, return the data, otherwise pass on to decode error.
	err = a.client.decode(&localVarReturnValue, localVarBody, localVarHttpResponse.Header.Get("Content-Type"));
	if err == nil { 
		return localVarReturnValue, localVarHttpResponse, err
	}
}

if localVarHttpResponse.StatusCode >= 300 {
	newErr := GenericSwaggerError{
		body: localVarBody,
		error: localVarHttpResponse.Status,
	}
	//the issue is here
	if localVarHttpResponse.StatusCode == 200 {
		var v V1beta1BatchCreateNotesResponse
		err = a.client.decode(&v, localVarBody, localVarHttpResponse.Header.Get("Content-Type"));
			if err != nil {
				newErr.error = err.Error()
				return localVarReturnValue, localVarHttpResponse, newErr
			}
			newErr.model = v
			return localVarReturnValue, localVarHttpResponse, newErr
	}
	
	return localVarReturnValue, localVarHttpResponse, newErr
}

This code checks if the StatusCode value is more than 300, and, if it is true, it checks if the StatusCode is equal to 200, which means that this nested if statement will never be true.

I did not observe any issues happening because of this problem because if StatusCode == 200 the if statement above (if localVarHttpResponse.StatusCode < 300 ) will be executed.

This problem reoccurs for every function in api_grafeas_v1_beta1.go that expects a response.

Potential Solution

Move StatusCode == 200 if statement inside of if localVarHttpResponse.StatusCode < 300 {} or before it, e.g.:

if localVarHttpResponse.StatusCode < 300 {
	// If we succeed, return the data, otherwise pass on to decode error.
	err = a.client.decode(&localVarReturnValue, localVarBody, localVarHttpResponse.Header.Get("Content-Type"));
	if localVarHttpResponse.StatusCode == 200 {
		if err != nil {
			newErr.error = err.Error()
			return localVarReturnValue, localVarHttpResponse, newErr
		}
		newErr.model = localVarReturnValue
		return localVarReturnValue, localVarHttpResponse, newErr
	}
	
	if err == nil { 
		return localVarReturnValue, localVarHttpResponse, err
	}
}

if localVarHttpResponse.StatusCode >= 300 {
	newErr := GenericSwaggerError{
		body: localVarBody,
		error: localVarHttpResponse.Status,
	}
	
	return localVarReturnValue, localVarHttpResponse, newErr
}

[aysylu]

Hi @lynnsh,

Thanks for finding and filing the issue! You're absolutely right that the status check for 200 is never reached in that if-statement. This code was auto-generated by Swagger, so we have 2 potential solutions here:

1. Fix locally inside the generated code. This would be a great approach, but if we decide to pursue longer-term fixes for Swagger in the Grafeas repo itself, as discussed in #379, then these changes would unfortunately get overwritten on the next client generation.

2. Fix this generation logic inside the Swagger codegen itself.

I personally would love to see the fix in the codegen, as it'd help many OSS projects, so if you decide to do 2. -- many thanks! :)