Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

panic: values with embedded slices uncomparable errors #29

Closed
stevvooe opened this issue May 24, 2016 · 3 comments
Closed

panic: values with embedded slices uncomparable errors #29

stevvooe opened this issue May 24, 2016 · 3 comments

Comments

@stevvooe
Copy link

Please see the stack trace here: https://travis-ci.org/opencontainers/image-spec/builds/132403826, described by opencontainers/image-spec#84.

Looks like #27 introduced or exposed a new error type that uses a slice and thus cannot be compared without introducing a panic. We picked this change up through an update to blackfriday, but this could happen anywhere.

While I understand that this library is based on concepts of no longer using sentinel values, this is already a common practice in existing code. There is also limited ways for a caller to avoid the panic.

I suggest that errors with a slice value use a pointer to the error value to avoid this trap in the future. For example, instead of building a stack{}, use &stack{}. This will ensure that existing code can make error comparisons while new code can begin to adopt this great package.

@davecheney
Copy link
Member

Thanks. I didn't consider that. I'll fix this immediately.

@stevvooe
Copy link
Author

@davecheney Cheers!

I had never seen this either...

davecheney added a commit that referenced this issue May 24, 2016
Fixed #29

errors.New, etc values are not expected to be compared by value but
the change in errors#27 made them incomparable. Assert that various
kinds of errors have a functional equality operator, even if the
result of that equality is always false.
davecheney added a commit that referenced this issue May 24, 2016
Fixed #29

errors.New, etc values are not expected to be compared by value but
the change in errors#27 made them incomparable. Assert that various
kinds of errors have a functional equality operator, even if the
result of that equality is always false.
@stevvooe
Copy link
Author

@davecheney Thanks for the quick fix!

YuJuncen pushed a commit to YuJuncen/errors that referenced this issue Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants