Repository: grpc/grpc-go
File: internal/xds/clients/xdsclient/channel.go (Go)

Code before:
// the resources.
//
// The returned map contains a key for each resource in the response, with the
// value being either the decoded resource data or an error if decoding failed.
// The returned metadata includes the version of the response, the timestamp of
// the update, and the status of the update (ACKed or NACKed).
//
// If there are any errors decoding the resources, the metadata will indicate
// that the update was NACKed, and the returned error will contain information
// about all errors encountered by this function.
func decodeResponse(opts *DecodeOptions, rType *ResourceType, resp response) (map[string]dataAndErrTuple, xdsresource.UpdateMetadata, error) {
	timestamp := time.Now()
	md := xdsresource.UpdateMetadata{
		Version:   resp.version,
		Timestamp: timestamp,
	}

	topLevelErrors := make([]error, 0)          // Tracks deserialization errors, where we don't have a resource name.
	perResourceErrors := make(map[string]error) // Tracks resource validation errors, where we have a resource name.
	ret := make(map[string]dataAndErrTuple)     // Return result, a map from resource name to either resource data or error.
	for _, r := range resp.resources {
		result, err := func() (res *DecodeResult, err error) {
			defer func() {
				if envconfig.XDSRecoverPanicInResourceParsing {
					if p := recover(); p != nil {
						err = fmt.Errorf("panic during %v resource decoding: %v\nstack: %s", rType.TypeName, p, debug.Stack())
					}
				}
			}()
			return rType.Decoder.Decode(NewAnyProto(r), *opts)
		}()

		// Name field of the result is left unpopulated only when resource
		// deserialization fails.
		name := ""
		if result != nil {
			name = xdsresource.ParseName(result.Name).String()
		}
		if err == nil {
			ret[name] = dataAndErrTuple{Resource: result.Resource}
			continue
		}
		if name == "" {
			topLevelErrors = append(topLevelErrors, err)
			continue
		}
		perResourceErrors[name] = err
		// Add place holder in the map so we know this resource name was in
		// the response.
		ret[name] = dataAndErrTuple{Err: xdsresource.NewError(xdsresource.ErrorTypeNACKed, err.Error())}
	}

Review comment (suggestion):
Nit: This should say "recovered from a panic" instead of just saying "panic".

Also, consider including the complete resource in the error instead of simply the resource type name.

Does the stack contain anything useful? Is it the stack from where the panic happened or will it return the stack from the current goroutine, in which case it is not going to be useful.

Code after:
//
// The returned map contains a key for each resource in the response, with the
// value being either the decoded resource data or an error if decoding failed.
// The returned metadata includes the version of the response, the timestamp of
// the update, and the status of the update (ACKed or NACKed).
//
// If there are any errors decoding the resources, the metadata will indicate
// that the update was NACKed, and the returned error will contain information
// about all errors encountered by this function.
func decodeResponse(opts *DecodeOptions, rType *ResourceType, resp response) (map[string]dataAndErrTuple, xdsresource.UpdateMetadata, error) {
	timestamp := time.Now()
	md := xdsresource.UpdateMetadata{
		Version:   resp.version,
		Timestamp: timestamp,
	}

	topLevelErrors := make([]error, 0)          // Tracks deserialization errors, where we don't have a resource name.
	perResourceErrors := make(map[string]error) // Tracks resource validation errors, where we have a resource name.
	ret := make(map[string]dataAndErrTuple)     // Return result, a map from resource name to either resource data or error.
	for _, r := range resp.resources {
		result, err := func() (res *DecodeResult, err error) {
			defer func() {
				if envconfig.XDSRecoverPanicInResourceParsing {
					if p := recover(); p != nil {
						err = fmt.Errorf("recovered from panic during resource parsing, resource: %v, panic: %v", r, p)
					}
				}
			}()
			return rType.Decoder.Decode(NewAnyProto(r), *opts)
		}()

		// Name field of the result is left unpopulated only when resource
		// deserialization fails.
		name := ""
		if result != nil {
			name = xdsresource.ParseName(result.Name).String()
		}
		if err == nil {
			ret[name] = dataAndErrTuple{Resource: result.Resource}
			continue
		}
		if name == "" {
			topLevelErrors = append(topLevelErrors, err)
			continue
		}
		perResourceErrors[name] = err
		// Add place holder in the map so we know this resource name was in
		// the response.
		ret[name] = dataAndErrTuple{Err: xdsresource.NewError(xdsresource.ErrorTypeNACKed, err.Error())}
	}