[BUG] Fix status error returned from rest code

[gnufied]

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?

[github-actions]

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.

[embano1]

@dougm this is also something I think we should address mid-term. Not sure about the impact on the APIs, thoughts?

[dougm]

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.

govmomi/vim25/soap/client.go

Lines 560 to 584 in 4a7de71

	type statusError struct {
	res *http.Response
	}
	// Temporary returns true for HTTP response codes that can be retried
	// See vim25.IsTemporaryNetworkError
	func (e *statusError) Temporary() bool {
	switch e.res.StatusCode {
	case http.StatusBadGateway:
	return true
	}
	return false
	}
	func (e *statusError) Error() string {
	return e.res.Status
	}
	func newStatusError(res *http.Response) error {
	return &url.Error{
	Op: res.Request.Method,
	URL: res.Request.URL.Path,
	Err: &statusError{res},
	}
	}

[brakthehack]

Interested in seeing this come about. A few more options:

1. 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.
2. Optionally, update all signatures to return this error.
3. 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.

[dougm]

1. 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.

2. 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.

3. 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

[github-actions]

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.

[brakthehack]

/remove-lifecycle stale

[github-actions]

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.

[embano1]

/remove-lifecycle stale

[github-actions]

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.

[brakthehack]

/remove-lifecycle stale