Confusing error messages with metric multierror type if wrapped is unset

[pkwarren]

Description

We're seeing some errors which I believe are coming from opentelemetry-go with the prefix %!w(<nil>): .

Environment

• OS: Linux
• Architecture: x86_64
• Go Version: 1.19.5
• opentelemetry-go version: v0.35.0 (Metrics)

Steps To Reproduce

1. Unknown how error is triggered (could be easily reproduced in a testcase however).

This appears to be the culprit:

• opentelemetry-go/sdk/metric/pipeline.go

  Line 507 in d5fca83

  errs := &multierror{}

  creates a multierror with no wrapped error.

• opentelemetry-go/sdk/metric/pipeline.go

  Line 527 in d5fca83

  return fmt.Errorf("%w: %s", m.wrapped, strings.Join(m.errors, "; "))

  is called.

Expected behavior

We shouldn't wrap errors if the error is nil. To fix, I think we should either:

1. Set m.wrapped if nil when calling func (m *multierror) append(err error).
2. Special case in errorOrNil() to remove the prefix %w: if wrapped is not set.

[ChillOrb]

@pkwarren

Hey, this could be because wrapper is not initialized like here

However, i was looking into %w from the go blog and feel %w here is used incorrectly. Please correct me if i am wrong.

Wrapping errors with %w

As mentioned earlier, it is common to use the fmt.Errorf function to add additional information to an error.

if err != nil {
return fmt.Errorf("decompress %v: %v", name, err)
}
In Go 1.13, the fmt.Errorf function supports a new %w verb. When this verb is present, the error returned by fmt.Errorf will have an Unwrap method returning the argument of %w, which must be an error. In all other ways, %w is identical to %v.

if err != nil {
// Return an error which unwraps to err.
return fmt.Errorf("decompress %v: %w", name, err)
}
Wrapping an error with %w makes it available to errors.Is and errors.As:

err := fmt.Errorf("access denied: %w", ErrPermission)
...
if errors.Is(err, ErrPermission) ...

Summing up,

• When you want to add additional context to an error without losing the original error information.
• When you want to chain multiple errors together to provide more detail about the error.
• When you want to wrap a specific error and modify the error message, but still have access to the original error information.

But here, we are using "errors []string" to append errors if we encounter one .. According to me we should either "append" or "chain"

If we were to implement this with %w we could do something like go playground

Please let me know what you think...
@MrAlias @Aneurysm9 Can you please share your views on this ?