Skip to content

Commit

Permalink
Improve situation around ErrSumNotSupported.
Browse files Browse the repository at this point in the history
The number that's missing is now reported in the error.
This is done using the golang error wrap feature.

We're keeping a constant value for ErrSumNotSupported for
compatibility and change avoidance reasons.

Code that cares about this exact error will still have to change;
it is now necessary to use `errors.Is` to detect it.

The text in the ErrSumNotSupported value is also updated,
because the text no longer made sense given an open registry system.
As a target-of-opportunity fix, it also now follows golang normative
conventions for error messages (no capitalization, no punctuation).
  • Loading branch information
warpfork committed Mar 10, 2021
1 parent 7be6719 commit cbd218c
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
5 changes: 3 additions & 2 deletions registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/sha1"
"crypto/sha256"
"crypto/sha512"
"fmt"
"hash"
)

Expand Down Expand Up @@ -50,11 +51,11 @@ func Register(indicator uint64, hasherFactory func() hash.Hash) {
// Other hash implementations can be made available by using the Register function.
// The 'go-mulithash/register/*' packages can also be imported to gain more common hash functions.
//
// If an error is returned, it will be ErrSumNotSupported.
// If an error is returned, it will match `errors.Is(err, ErrSumNotSupported)`.
func GetHasher(indicator uint64) (hash.Hash, error) {
factory, exists := registry[indicator]
if !exists {
return nil, ErrSumNotSupported // REVIEW: it's unfortunate that this error doesn't say what code was missing. Also "NotSupported" is a bit of a misnomer now.
return nil, fmt.Errorf("unknown multihash code %d (0x%x): %w", indicator, indicator, ErrSumNotSupported)
}
return factory(), nil
}
Expand Down
2 changes: 1 addition & 1 deletion sum.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
)

// ErrSumNotSupported is returned when the Sum function code is not implemented
var ErrSumNotSupported = errors.New("Function not implemented. Complain to lib maintainer.")
var ErrSumNotSupported = errors.New("no such hash registered")

var ErrLenTooLarge = errors.New("requested length was too large for digest")

Expand Down
6 changes: 4 additions & 2 deletions sum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package multihash_test
import (
"bytes"
"encoding/hex"
"errors"
"fmt"
"runtime"
"testing"
Expand Down Expand Up @@ -132,8 +133,9 @@ func TestTooLargeLength(t *testing.T) {
func TestBasicSum(t *testing.T) {
for code, name := range multihash.Codes {
_, err := multihash.Sum([]byte("test"), code, -1)
switch err {
case multihash.ErrSumNotSupported, nil:
switch {
case errors.Is(err, multihash.ErrSumNotSupported):
case err == nil:
default:
t.Errorf("unexpected error for %s: %s", name, err)
}
Expand Down

0 comments on commit cbd218c

Please sign in to comment.