govmomi
govmomi copied to clipboard
vmware
[BUG] Fix status error returned from rest code
Describe the bug
When creating tags or cateogies, the sdk returns following error type:
https://github.com/vmware/govmomi/blob/master/vapi/rest/client.go#L178
return &statusError{res}
The problem is - users of library generally want to know the http error code at very minimum, so as they can make reasonable assumption about error. Such as - did getting a resource failed because of network error or was it 404?
Howdy 🖐 gnufied ! Thank you for your interest in this project. We value your feedback and will respond soon.
If you want to contribute to this project, please make yourself familiar with the CONTRIBUTION guidelines.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@dougm this is also something I think we should address mid-term. Not sure about the impact on the APIs, thoughts?
Agreed, I had started to look at doing this a while back.. was thinking we could export the existing soap.statusError and also expose the http.Response.StatusCode. Then use that error type in the rest client too.
https://github.com/vmware/govmomi/blob/4a7de71a1d515a62ecf5a62c043e7e3859359b13/vim25/soap/client.go#L560-L584
Interested in seeing this come about. A few more options:
- Make statusError public and have it carry core parts of the request such status code. If we expose it, then clients can at least cast to that error and grab the response information when they need to.
- Optionally, update all signatures to return this error.
- For operations that return an object with a pointer, make the pointer nil when 404 is encountered. Seems there are similar issues which report that this should be expected behavior: https://github.com/vmware/govmomi/issues/1639.
- Make statusError public and have it carry core parts of the request such status code. If we expose it, then clients can at least cast to that error and grab the response information when they need to.
Agreed, this is a must have.
- Optionally, update all signatures to return this error.
I think we still want to return the error interface type. There are errors the Go http client may return, such as network errors than can happen before an HTTP request is sent.
- For operations that return an object with a pointer, make the pointer nil when 404 is encountered. Seems there are similar issues which report that this should be expected behavior: Weird 404 behavior with tags.GetCategory() when no category exists #1639.
Yes, need to revisit that too
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Mark as fresh by adding the comment /remove-lifecycle stale.
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Mark as fresh by adding the comment /remove-lifecycle stale.
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Mark as fresh by adding the comment /remove-lifecycle stale.