Action functions that return bool, error should just return error #40
Description
There are no cases where the request to perform some action in the Linode API should return false, nil on a successful API request.
Code using the current convention tends to look like:
if ok, err := client.DoThing(); err != nil {
return fmt.Errorf("Error doing thing: %s", err)
} else ! ok {
return fmt.Errorf("Error doing thing: thing didn't happen")
}For example, RebootInstance has no meaningful false state.
Convert all of these endpoints to simply return error. If there is a case where false makes sense, consider if it should just be denoted with:
- a code comment
// BootInstance will not return an error if the Instance is already booted
I'm not sure about that, just an example - or with a simulated error
return fmt.Errorf("Instance was already booted")
Another option may be to use a linodego.Error, introducing a Code (<100) to indicate that there was no API error but the request was not performed for a predictable reason as returned by the API (usually in the case of a a non HTTP 200, non HTTP 4xx response).
settleBoolResponseOrError can likely be removed as a result of this refactor.
git grep 'bool, error' | sed -E -e 's/func \(c \*Client\)//' -e 's/\(.*//':
- instance_snapshots.go:
EnableInstanceBackups - instance_snapshots.go:
CancelInstanceBackups - instance_snapshots.go:
RestoreInstanceBackup - instances.go:
BootInstance - instances.go:
RebootInstance - instances.go:
MutateInstance - instances.go:
RescueInstance - instances.go:
ResizeInstance - instances.go:
ShutdownInstance - volumes.go:
DetachVolume - volumes.go:
ResizeVolume
This should be denoted as a "BREAKING CHANGE" in the git commit.