Bug checking response status

[haraldfuchs]

There's a serious bug in almost all files - here's an example:

res, err := c.DoRequest(r)
if err != nil {
return ServiceInstance{}, err
}

defer res.Body.Close()
if res.StatusCode != http.StatusAccepted && res.StatusCode != http.StatusCreated {
return ServiceInstance{}, errors.Wrapf(err, "Error creating service, response code: %d", res.StatusCode)
}

By the time errors.Wrapf is called it's clear that err is nil because of the previous if err != nil { clause. If you call errors.Wrapf with nil as a first argument, it will return nil. This means that whenever the Cloud Controller returns an unexpected status code, the error message will be lost, and no error will be returned to the caller.

I suggest replacing errors.Wrapf by fmt.Errorf or errors.Errorf in all those places.

[haraldfuchs]

When you try to fix that, you'll notice that TestUnshareOrgPrivateDomain and TestCreateServiceKey start to fail. These tests did't fail previously because of the bug - the tests themselves are wrong.

[sneal]

Fixed in v3 branch

[haraldfuchs]

Thanks for the nice work!  Two minor typos in service_plan.go:

ServicePlanGUIDs     Filter `filter:"service_offering_guids,omitempty"`
ServicePlanNames     Filter `filter:"service_offering_names,omitempty"`

Shouldn't that be ServiceOfferingGUIDs and ServiceOfferingNames?

On 11/8/22 23:43, Shawn Neal wrote:

Fixed in v3 branch

— Reply to this email directly, view it on GitHub https://github.com/cloudfoundry-community/go-cfclient/issues/315#issuecomment-1307937420, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCKVMKT4VLHXHTZF3U7QY3WHLJQLANCNFSM56RHB7JQ. You are receiving this because you authored the thread.

Message ID: @.***>

[sneal]

@haraldfuchs I appreciate you taking a look. Nice catch! Fixed locally.

[haraldfuchs]

Glad to have been be helpful.  Besides the two typos, everything I need seems to work fine - except for two things:

1. How to reference the V3 stuff in a Go module? go mod tidy complains:

go: finding module for package 
github.com/cloudfoundry-community/go-cfclient/resource
go: finding module for package 
github.com/cloudfoundry-community/go-cfclient/client
github.tools.sap/perfx/cftest imports
         github.com/cloudfoundry-community/go-cfclient/client: module 
***@***.*** found 
(v0.0.0-20220930021109-9c4e6c59ccf1), but does not contain package 
github.com/cloudfoundry-community/go-cfclient/client
github.tools.sap/perfx/cftest imports
         github.com/cloudfoundry-community/go-cfclient/resource: module 
***@***.*** found 
(v0.0.0-20220930021109-9c4e6c59ccf1), but does not contain package 
github.com/cloudfoundry-community/go-cfclient/resource

2. I'd like to avoid ListAll and rather iterate over the result pages.  I guess I need to call List and use a Pager for that. Is there any example how to do that?

On 11/10/22 17:28, Shawn Neal wrote:

@haraldfuchs https://github.com/haraldfuchs I appreciate you taking a look. Nice catch! Fixed locally.

— Reply to this email directly, view it on GitHub https://github.com/cloudfoundry-community/go-cfclient/issues/315#issuecomment-1310560026, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCKVMLOCYNGNGXSU2AMKBTWHUPD7ANCNFSM56RHB7JQ. You are receiving this because you were mentioned.Message ID: @.***>

[sneal]

Hey @haraldfuchs, the error contains github.com/cloudfoundry-community/go-cfclient/resource, that looks wrong as it should contain v3 in the go.mod via go get and import statements. The import statement should look like github.com/cloudfoundry-community/go-cfclient/v3/resource. Here's a working consuming application for comparison.

All resource collections support pagination, take a look at the Pagination section in the README.md on to iterate over result pages. Although not currently in the docs the ListOptions allow you to configure the page size if needed.