akutz
commented
2 months ago @dougm I will add an As function before we merge this.
Closed akutz closed 2 months ago
akutz
commented
2 months ago @dougm I will add an As function before we merge this.
dougm
commented
2 months ago Noting related issues on this topic: https://github.com/vmware/govmomi/issues/2048 https://github.com/vmware/govmomi/issues/2685
Personally am still in favor of just adding fault.Is and fault.As, rather than adding generated methods for all fault types to support errors.Is and errors.As. Though maybe there's a simpler way, been a while since I looked.
akutz
commented
2 months ago Sweet, thanks @akutz I was trying it out before I saw your comment about adding
Asafter merge. Looking forward to that!if fault.Is(err, &types.InvalidPowerState{}) { }func Is(err error, target types.BaseMethodFault) bool { kind := reflect.TypeOf(target).Elem() is := false InError(err, func(fault types.BaseMethodFault, _ string, _ []types.LocalizableMessage) bool { source := reflect.TypeOf(fault).Elem() if kind == source { is = true } return is }) return is }
I just added the As function.
akutz
commented
2 months ago @dougm Please note, I did not add Is yet because the question is what we match. Should it be just the fault type? Should it be the fault data? Should the fault data include all of the localized message properties? I think we should merge this PR and then add fault.Is later after we think more about what it should be. Because otherwise, Is just becomes a wrapper for As. The stdlib errors.Is just compares the type, so if we want to do the same, I can do that real quick. Thoughts?
dougm
commented
2 months ago Because otherwise,
Isjust becomes a wrapper forAs. The stdliberrors.Isjust compares the type, so if we want to do the same, I can do that real quick. Thoughts?
Yes, that's what I was thinking. A few examples I'd like to replace with fault.Is:
https://github.com/vmware/govmomi/blob/d3cb5c6df6cc08f044ce65fe2992026e8fc7b632/govc/datastore/ls.go#L77-L86
There's more of those, and I can take care of replacing them, just pointing out some use cases.
akutz
commented
2 months ago Okay @dougm , go nuts! There are now just three functions: As, In, and Is. Please read the GoDoc, tests, and examples for As carefully. I constructed both As and Is to behave exactly as their stdlib counterparts, and this means that As accepts a target type of **types.SystemError (as an example) instead of *types.SystemError. This is because you must send in the address of an object that is the type that implements types.BaseMethodFault. Since *types.SystemError implements types.BaseMethodFault, it is necessary to send in **types.SystemError as the target.
There are Golang-style examples showing both.
akutz
commented
2 months ago @dougm You will love it even more -- I just updated the PR to support any types that implement Fault() types.BaseMethodFault, which now includes errors wrapped with soap.ToSoapFault (if there is an internal VIM error) and soap.ToVimFault.
Description
This patch adds support for the new, top-level
faultpackage with helper functions such asAs,In, andIsfor testing and traversing Golangerrortypes and VMwarefaulttypes.Closes:
NAType of change
Please mark options that are relevant:
How Has This Been Tested?
With the command:
The results shows there is 100% code coverage:
Checklist:
CONTRIBUTIONguidelines of this project