Refactor errors to use pointer receivers

theupdateframework · Feb 5, 2024 · 06838e4 · 06838e4
1 parent a1a9c88
commit 06838e4
Show file tree

Hide file tree

Showing 11 changed files with 271 additions and 201 deletions.
diff --git a/metadata/errors.go b/metadata/errors.go
@@ -34,78 +34,116 @@ type ErrRepository struct {
 	Msg string
 }
 
-func (e ErrRepository) Error() string {
+func (e *ErrRepository) Error() string {
 	return fmt.Sprintf("repository error: %s", e.Msg)
 }
 
+func (e *ErrRepository) Is(target error) bool {
+	_, ok := target.(*ErrRepository)
+	return ok
+}
+
 // ErrUnsignedMetadata - An error about metadata object with insufficient threshold of signatures
 type ErrUnsignedMetadata struct {
 	Msg string
 }
 
-func (e ErrUnsignedMetadata) Error() string {
+func (e *ErrUnsignedMetadata) Error() string {
 	return fmt.Sprintf("unsigned metadata error: %s", e.Msg)
 }
 
 // ErrUnsignedMetadata is a subset of ErrRepository
-func (e ErrUnsignedMetadata) Is(target error) bool {
-	return target == ErrRepository{} || target == ErrUnsignedMetadata{}
+func (e *ErrUnsignedMetadata) Is(target error) bool {
+	if _, ok := target.(*ErrUnsignedMetadata); ok {
+		return true
+	}
+	if _, ok := target.(*ErrRepository); ok {
+		return true
+	}
+	return false
 }
 
 // ErrBadVersionNumber - An error for metadata that contains an invalid version number
 type ErrBadVersionNumber struct {
 	Msg string
 }
 
-func (e ErrBadVersionNumber) Error() string {
+func (e *ErrBadVersionNumber) Error() string {
 	return fmt.Sprintf("bad version number error: %s", e.Msg)
 }
 
 // ErrBadVersionNumber is a subset of ErrRepository
-func (e ErrBadVersionNumber) Is(target error) bool {
-	return target == ErrRepository{} || target == ErrBadVersionNumber{}
+func (e *ErrBadVersionNumber) Is(target error) bool {
+	if _, ok := target.(*ErrBadVersionNumber); ok {
+		return true
+	}
+	if _, ok := target.(*ErrRepository); ok {
+		return true
+	}
+	return false
 }
 
 // ErrEqualVersionNumber - An error for metadata containing a previously verified version number
 type ErrEqualVersionNumber struct {
 	Msg string
 }
 
-func (e ErrEqualVersionNumber) Error() string {
+func (e *ErrEqualVersionNumber) Error() string {
 	return fmt.Sprintf("equal version number error: %s", e.Msg)
 }
 
 // ErrEqualVersionNumber is a subset of both ErrRepository and ErrBadVersionNumber
-func (e ErrEqualVersionNumber) Is(target error) bool {
-	return target == ErrRepository{} || target == ErrBadVersionNumber{} || target == ErrEqualVersionNumber{}
+func (e *ErrEqualVersionNumber) Is(target error) bool {
+	if _, ok := target.(*ErrEqualVersionNumber); ok {
+		return true
+	}
+	if _, ok := target.(*ErrBadVersionNumber); ok {
+		return true
+	}
+	if _, ok := target.(*ErrRepository); ok {
+		return true
+	}
+	return false
 }
 
 // ErrExpiredMetadata - Indicate that a TUF Metadata file has expired
 type ErrExpiredMetadata struct {
 	Msg string
 }
 
-func (e ErrExpiredMetadata) Error() string {
+func (e *ErrExpiredMetadata) Error() string {
 	return fmt.Sprintf("expired metadata error: %s", e.Msg)
 }
 
 // ErrExpiredMetadata is a subset of ErrRepository
-func (e ErrExpiredMetadata) Is(target error) bool {
-	return target == ErrRepository{} || target == ErrExpiredMetadata{}
+func (e *ErrExpiredMetadata) Is(target error) bool {
+	if _, ok := target.(*ErrExpiredMetadata); ok {
+		return true
+	}
+	if _, ok := target.(*ErrRepository); ok {
+		return true
+	}
+	return false
 }
 
 // ErrLengthOrHashMismatch - An error while checking the length and hash values of an object
 type ErrLengthOrHashMismatch struct {
 	Msg string
 }
 
-func (e ErrLengthOrHashMismatch) Error() string {
+func (e *ErrLengthOrHashMismatch) Error() string {
 	return fmt.Sprintf("length/hash verification error: %s", e.Msg)
 }
 
 // ErrLengthOrHashMismatch is a subset of ErrRepository
-func (e ErrLengthOrHashMismatch) Is(target error) bool {
-	return target == ErrRepository{} || target == ErrLengthOrHashMismatch{}
+func (e *ErrLengthOrHashMismatch) Is(target error) bool {
+	if _, ok := target.(*ErrLengthOrHashMismatch); ok {
+		return true
+	}
+	if _, ok := target.(*ErrRepository); ok {
+		return true
+	}
+	return false
 }
 
 // Download errors
@@ -115,22 +153,33 @@ type ErrDownload struct {
 	Msg string
 }
 
-func (e ErrDownload) Error() string {
+func (e *ErrDownload) Error() string {
 	return fmt.Sprintf("download error: %s", e.Msg)
 }
 
+func (e *ErrDownload) Is(target error) bool {
+	_, ok := target.(*ErrDownload)
+	return ok
+}
+
 // ErrDownloadLengthMismatch - Indicate that a mismatch of lengths was seen while downloading a file
 type ErrDownloadLengthMismatch struct {
 	Msg string
 }
 
-func (e ErrDownloadLengthMismatch) Error() string {
+func (e *ErrDownloadLengthMismatch) Error() string {
 	return fmt.Sprintf("download length mismatch error: %s", e.Msg)
 }
 
 // ErrDownloadLengthMismatch is a subset of ErrDownload
-func (e ErrDownloadLengthMismatch) Is(target error) bool {
-	return target == ErrDownload{} || target == ErrDownloadLengthMismatch{}
+func (e *ErrDownloadLengthMismatch) Is(target error) bool {
+	if _, ok := target.(*ErrDownloadLengthMismatch); ok {
+		return true
+	}
+	if _, ok := target.(*ErrDownload); ok {
+		return true
+	}
+	return false
 }
 
 // ErrDownloadHTTP - Returned by Fetcher interface implementations for HTTP errors
@@ -139,38 +188,59 @@ type ErrDownloadHTTP struct {
 	URL        string
 }
 
-func (e ErrDownloadHTTP) Error() string {
+func (e *ErrDownloadHTTP) Error() string {
 	return fmt.Sprintf("failed to download %s, http status code: %d", e.URL, e.StatusCode)
 }
 
 // ErrDownloadHTTP is a subset of ErrDownload
-func (e ErrDownloadHTTP) Is(target error) bool {
-	return target == ErrDownload{} || target == ErrDownloadHTTP{}
+func (e *ErrDownloadHTTP) Is(target error) bool {
+	if _, ok := target.(*ErrDownloadHTTP); ok {
+		return true
+	}
+	if _, ok := target.(*ErrDownload); ok {
+		return true
+	}
+	return false
 }
 
 // ValueError
 type ErrValue struct {
 	Msg string
 }
 
-func (e ErrValue) Error() string {
+func (e *ErrValue) Error() string {
 	return fmt.Sprintf("value error: %s", e.Msg)
 }
 
+func (e *ErrValue) Is(err error) bool {
+	_, ok := err.(*ErrValue)
+	return ok
+}
+
 // TypeError
 type ErrType struct {
 	Msg string
 }
 
-func (e ErrType) Error() string {
+func (e *ErrType) Error() string {
 	return fmt.Sprintf("type error: %s", e.Msg)
 }
 
+func (e *ErrType) Is(err error) bool {
+	_, ok := err.(*ErrType)
+	return ok
+}
+
 // RuntimeError
 type ErrRuntime struct {
 	Msg string
 }
 
-func (e ErrRuntime) Error() string {
+func (e *ErrRuntime) Error() string {
 	return fmt.Sprintf("runtime error: %s", e.Msg)
 }
+
+func (e *ErrRuntime) Is(err error) bool {
+	_, ok := err.(*ErrRuntime)
+	return ok
+}
diff --git a/metadata/fetcher/fetcher.go b/metadata/fetcher/fetcher.go
@@ -57,7 +57,7 @@ func (d *DefaultFetcher) DownloadFile(urlPath string, maxLength int64, timeout t
 	defer res.Body.Close()
 	// Handle HTTP status codes.
 	if res.StatusCode == http.StatusNotFound || res.StatusCode == http.StatusForbidden || res.StatusCode != http.StatusOK {
-		return nil, metadata.ErrDownloadHTTP{StatusCode: res.StatusCode, URL: urlPath}
+		return nil, &metadata.ErrDownloadHTTP{StatusCode: res.StatusCode, URL: urlPath}
 	}
 	var length int64
 	// Get content length from header (might not be accurate, -1 or not set).
@@ -68,7 +68,7 @@ func (d *DefaultFetcher) DownloadFile(urlPath string, maxLength int64, timeout t
 		}
 		// Error if the reported size is greater than what is expected.
 		if length > maxLength {
-			return nil, metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
+			return nil, &metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
 		}
 	}
 	// Although the size has been checked above, use a LimitReader in case
@@ -82,7 +82,7 @@ func (d *DefaultFetcher) DownloadFile(urlPath string, maxLength int64, timeout t
 	// Error if the reported size is greater than what is expected.
 	length = int64(len(data))
 	if length > maxLength {
-		return nil, metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
+		return nil, &metadata.ErrDownloadLengthMismatch{Msg: fmt.Sprintf("download failed for %s, length %d is larger than expected %d", urlPath, length, maxLength)}
 	}
 
 	return data, nil

diff --git a/metadata/fetcher/fetcher_test.go b/metadata/fetcher/fetcher_test.go
@@ -64,15 +64,15 @@ func TestDownLoadFile(t *testing.T) {
 			desc:    "Path does not exist",
 			url:     "https://jku.github.io/tuf-demo/metadata/badPath.json",
 			data:    nil,
-			wantErr: metadata.ErrDownloadHTTP{},
+			wantErr: &metadata.ErrDownloadHTTP{},
 		},
 		{
 			name:      "data too long",
 			desc:      "Returned data is longer than maxLength",
 			url:       "https://jku.github.io/tuf-demo/metadata/1.root.json",
 			maxLength: 1,
 			data:      nil,
-			wantErr:   metadata.ErrDownloadLengthMismatch{},
+			wantErr:   &metadata.ErrDownloadLengthMismatch{},
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/metadata/marshal.go b/metadata/marshal.go
@@ -335,7 +335,7 @@ func (meta *Metadata[T]) UnmarshalJSON(data []byte) error {
 		meta.Signed = i.(T)
 		meta.Signatures = dict.Signatures
 	default:
-		return ErrValue{Msg: "unrecognized metadata type"}
+		return &ErrValue{Msg: "unrecognized metadata type"}
 	}
 	delete(m, "signed")
 	delete(m, "signatures")
@@ -470,7 +470,7 @@ func (role DelegatedRole) MarshalJSON() ([]byte, error) {
 	dict["terminating"] = role.Terminating
 	// make sure we have only one of the two (per spec)
 	if role.Paths != nil && role.PathHashPrefixes != nil {
-		return nil, ErrValue{Msg: "failed to marshal: not allowed to have both \"paths\" and \"path_hash_prefixes\" present"}
+		return nil, &ErrValue{Msg: "failed to marshal: not allowed to have both \"paths\" and \"path_hash_prefixes\" present"}
 	}
 	if role.Paths != nil {
 		dict["paths"] = role.Paths