google-cloud-go: compute: service account-related BulkInsert error not surfaced to user

I’m hoping we can get it in this week’s release, but if not this week then definitely next.

Ok so we have the initial changes merged into main here: https://github.com/googleapis/google-cloud-go/blob/151908ab02a0b22c8e537d1bee00afc7f1465511/compute/apiv1/operations.go#L192-L199

We want to get a test case into our integration tests, and waiting on finding a good error mode to reliably test with.

I’m hoping we can release this code next week. Feel free to try it yourself though if you want to wire up the unreleased dependency.

I just hit this today while trying to delete a network that still had a subnetwork in it, and the error was undetected. I’ve noticed that the workaround of checking the http status doesn’t work if the operation succeeded (at least in the network deletion case) because that field is not present in the result. So you have to check if it’s there at all, and then check it against http.StatusOK.

Here’s my temporary workaround:

// ComputeOperationError is a wrapper around a compute.Operation that has
// errored out.
//
// TODO (https://github.com/googleapis/google-cloud-go/issues/7876): Remove once this is fixed.
type ComputeOperationError struct {
	Op *computepb.Operation
}

func (e *ComputeOperationError) Error() string {
	if e.Op.GetStatus() != computepb.Operation_DONE {
		return fmt.Sprintf("operation %q is not done", e.Op.GetName())
	}
	if len(e.Op.GetError().GetErrors()) > 0 && len(e.Op.GetError().GetErrors()[0].GetMessage()) > 0 {
		return e.Op.GetError().Errors[0].GetMessage()
	}
	return e.Op.GetHttpErrorMessage()
}

// ErrorFromComputeOperation returns an error if the compute.Operation has
// errored out. Otherwise, it returns nil.
func ErrorFromComputeOperation(op *compute.Operation) error {
	if op.Proto().HttpErrorStatusCode != nil && op.Proto().GetHttpErrorStatusCode() != http.StatusOK {
		return &ComputeOperationError{op.Proto()}
	}
	return nil
}

Well, you nailed it when you pointed out Compute Engine uses its own Operations client, not the longrunning one. 😃 This is getting in the weeds, and frankly as user, you shouldn’t have to care about this difference, but because pretty much every other Cloud API uses LRO, we are able to generate highly regular code, especially when that code requires inspecting the response in some way (e.g. checking if the Operation.result is an error). Since Compute spun its own Operations pattern, with its own types and services, we had to start annotating the schema with indicators of what each element means relative to something like LRO.

So, this was just a mistake on our part while implementing generator support. We left out this admittedly crucial piece.

@ijrsvt you’re right, but we can’t use your suggestion directly. I’ve filed https://github.com/googleapis/gapic-generator-go/issues/1320 to fix this in our client generator.

We can’t use h.proto.Error != nil directly because the Compute Operation.error field is “non standard” vs. other APIs. The http_error_message and http_error_status_code fields, however, are simple to consume and annotated as the “error code” and “error message” fields. Other non-standard APIs will have similar such fields annotated in the same way. We can use these to generically construct an error type.

@hanming-lu in the mean time, you can do something similar to what @ijrsvt suggests and check the error via op.Proto().GetHttpErrorStatusCode() != http.StatusOK. I know this isn’t ideal and we will work to get this rectified ASAP.