Add errorlint and fix all existing issues (knative#9955)

* Add errorlint and fix all existing issues

* Fix wrong check

.golangci.yaml

@@ -19,6 +19,7 @@ run:
 linters:
   enable:
     - asciicheck
+    - errorlint
     - golint
     - gosec
     - prealloc

cmd/activator/main.go

@@ -219,7 +219,7 @@ func main() {
 	for name, server := range servers {
 		go func(name string, s *http.Server) {
 			// Don't forward ErrServerClosed as that indicates we're already shutting down.
-			if err := s.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+			if err := s.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 				errCh <- fmt.Errorf("%s server failed: %w", name, err)
 			}
 		}(name, server)

cmd/autoscaler/main.go

@@ -19,6 +19,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -182,7 +183,7 @@ func main() {
 	statsServer.Shutdown(5 * time.Second)
 	profilingServer.Shutdown(context.Background())
 	// Don't forward ErrServerClosed as that indicates we're already shutting down.
-	if err := eg.Wait(); err != nil && err != http.ErrServerClosed {
+	if err := eg.Wait(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 		logger.Errorw("Error while running server", zap.Error(err))
 	}
 }

cmd/queue/main.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"net"
@@ -195,7 +196,7 @@ func main() {
 			}
 
 			// Don't forward ErrServerClosed as that indicates we're already shutting down.
-			if err := s.Serve(l); err != nil && err != http.ErrServerClosed {
+			if err := s.Serve(l); err != nil && !errors.Is(err, http.ErrServerClosed) {
 				errCh <- fmt.Errorf("%s server failed to serve: %w", name, err)
 			}
 		}(name, server)

pkg/activator/handler/handler.go

@@ -18,6 +18,7 @@ package handler
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -94,10 +95,9 @@ func (a *activationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 		logger.Errorw("Throttler try error", zap.Error(err))
 
-		switch err {
-		case context.DeadlineExceeded, queue.ErrRequestQueueFull:
+		if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, queue.ErrRequestQueueFull) {
 			http.Error(w, err.Error(), http.StatusServiceUnavailable)
-		default:
+		} else {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

pkg/activator/net/throttler_test.go

@@ -243,7 +243,7 @@ func TestThrottlerErrorNoRevision(t *testing.T) {
 
 	// Make sure errors are propagated correctly.
 	innerError := errors.New("inner")
-	if err := throttler.Try(ctx, func(string) error { return innerError }); err != innerError {
+	if err := throttler.Try(ctx, func(string) error { return innerError }); !errors.Is(err, innerError) {
 		t.Fatalf("Try() = %v, want %v", err, innerError)
 	}
 
@@ -299,7 +299,7 @@ func TestThrottlerErrorOneTimesOut(t *testing.T) {
 	})
 
 	// The first result will be a timeout because of the locking logic.
-	if result := <-resultChan; result.err != context.DeadlineExceeded {
+	if result := <-resultChan; !errors.Is(result.err, context.DeadlineExceeded) {
 		t.Fatalf("err = %v, want %v", err, context.DeadlineExceeded)
 	}
 
@@ -811,7 +811,7 @@ func TestMultipleActivators(t *testing.T) {
 	})
 
 	// The first result will be a timeout because of the locking logic.
-	if result := <-resultChan; result.err != context.DeadlineExceeded {
+	if result := <-resultChan; !errors.Is(result.err, context.DeadlineExceeded) {
 		t.Fatalf("err = %v, want %v", err, context.DeadlineExceeded)
 	}
 

pkg/apis/autoscaling/annotation_validation.go

@@ -18,6 +18,7 @@ package autoscaling
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math"
 	"strconv"
@@ -36,7 +37,7 @@ func getIntGE0(m map[string]string, k string) (int32, *apis.FieldError) {
 	// Parsing as uint gives a bad format error, rather than invalid range, unfortunately.
 	i, err := strconv.ParseInt(v, 10, 32)
 	if err != nil {
-		if nerr, ok := err.(*strconv.NumError); ok && nerr.Err == strconv.ErrRange {
+		if errors.Is(err, strconv.ErrRange) {
 			return 0, apis.ErrOutOfBoundsValue(v, 0, math.MaxInt32, k)
 		}
 		return 0, apis.ErrInvalidValue(v, k)

pkg/autoscaler/metrics/collector.go

@@ -340,7 +340,7 @@ func (c *collection) updateLastError(err error) bool {
 	c.mux.Lock()
 	defer c.mux.Unlock()
 
-	if c.lastErr == err {
+	if errors.Is(err, c.lastErr) {
 		return false
 	}
 	c.lastErr = err

pkg/autoscaler/metrics/collector_test.go

@@ -78,7 +78,7 @@ func TestMetricCollectorCRUD(t *testing.T) {
 		coll := NewMetricCollector(failingFactory, logger)
 		got := coll.CreateOrUpdate(&defaultMetric)
 
-		if got != want {
+		if !errors.Is(got, want) {
 			t.Errorf("Create() = %v, want %v", got, want)
 		}
 	})
@@ -284,10 +284,10 @@ func TestMetricCollectorScraper(t *testing.T) {
 
 	// Deleting the metric should cause a calculation error.
 	coll.Delete(defaultNamespace, defaultName)
-	if _, _, err := coll.StableAndPanicConcurrency(metricKey, now); err != ErrNotCollecting {
+	if _, _, err := coll.StableAndPanicConcurrency(metricKey, now); !errors.Is(err, ErrNotCollecting) {
 		t.Errorf("StableAndPanicConcurrency() = %v, want %v", err, ErrNotCollecting)
 	}
-	if _, _, err := coll.StableAndPanicRPS(metricKey, now); err != ErrNotCollecting {
+	if _, _, err := coll.StableAndPanicRPS(metricKey, now); !errors.Is(err, ErrNotCollecting) {
 		t.Errorf("StableAndPanicRPS() = %v, want %v", err, ErrNotCollecting)
 	}
 }
@@ -392,10 +392,10 @@ func TestMetricCollectorNoDataError(t *testing.T) {
 	// Verify correct error is returned if ScrapeTarget is set
 	_, _, errCon := coll.StableAndPanicConcurrency(metricKey, now)
 	_, _, errRPS := coll.StableAndPanicRPS(metricKey, now)
-	if errCon != ErrNoData {
+	if !errors.Is(errCon, ErrNoData) {
 		t.Error("StableAndPanicConcurrency() =", errCon)
 	}
-	if errRPS != ErrNoData {
+	if !errors.Is(errRPS, ErrNoData) {
 		t.Error("StableAndPanicRPS() =", errRPS)
 	}
 }
@@ -562,7 +562,7 @@ func TestMetricCollectorError(t *testing.T) {
 			}
 
 			// Make sure the error is surfaced via 'CreateOrUpdate', which is called in the reconciler.
-			if err := coll.CreateOrUpdate(testMetric); err != test.expectedError {
+			if err := coll.CreateOrUpdate(testMetric); !errors.Is(err, test.expectedError) {
 				t.Fatalf("CreateOrUpdate = %v, want %v", err, test.expectedError)
 			}
 

pkg/autoscaler/metrics/stats_scraper.go

@@ -207,7 +207,7 @@ func (s *serviceScraper) Scrape(window time.Duration) (stat Stat, err error) {
 	if s.podsAddressable {
 		stat, err := s.scrapePods(window)
 		// Some pods were scraped, but not enough.
-		if err != errNoPodsScraped {
+		if !errors.Is(err, errNoPodsScraped) {
 			return stat, err
 		}
 		// Else fall back to service scrape.
@@ -385,7 +385,7 @@ func (s *serviceScraper) scrapeService(window time.Duration, readyPods int) (Sta
 	// Return the inner error, if any.
 	if err := grp.Wait(); err != nil {
 		// Ignore the error if we have received enough statistics.
-		if err != ErrDidNotReceiveStat || len(oldStatCh)+len(youngStatCh) < sampleSize {
+		if !errors.Is(err, ErrDidNotReceiveStat) || len(oldStatCh)+len(youngStatCh) < sampleSize {
 			return emptyStat, fmt.Errorf("unsuccessful scrape, sampleSize=%d: %w", sampleSize, err)
 		}
 	}

pkg/autoscaler/scaling/autoscaler.go

@@ -18,6 +18,7 @@ package scaling
 
 import (
 	"context"
+	"errors"
 	"math"
 	"sync"
 	"time"
@@ -172,7 +173,7 @@ func (a *autoscaler) Scale(ctx context.Context, now time.Time) ScaleResult {
 	logger = logger.With(zap.String("metric", metricName))
 
 	if err != nil {
-		if err == metrics.ErrNoData {
+		if errors.Is(err, metrics.ErrNoData) {
 			logger.Debug("No data to scale on yet")
 		} else {
 			logger.Errorw("Failed to obtain metrics", zap.Error(err))

pkg/autoscaler/statserver/server.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"net"
 	"net/http"
 	"strings"
@@ -100,7 +101,7 @@ func (s *Server) listen() (net.Listener, error) {
 
 func (s *Server) serve(l net.Listener) error {
 	close(s.servingCh)
-	if err := s.wsSrv.Serve(l); err != http.ErrServerClosed {
+	if err := s.wsSrv.Serve(l); !errors.Is(err, http.ErrServerClosed) {
 		return err
 	}
 	return nil
@@ -221,7 +222,7 @@ func (s *Server) Shutdown(timeout time.Duration) {
 	defer cancel()
 	err := s.wsSrv.Shutdown(ctx)
 	if err != nil {
-		if err == context.DeadlineExceeded {
+		if errors.Is(err, context.DeadlineExceeded) {
 			s.logger.Warn("Shutdown timed out")
 		} else {
 			s.logger.Errorw("Shutdown failed.", zap.Error(err))

pkg/autoscaler/statserver/server_test.go

@@ -19,6 +19,7 @@ package statserver
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -142,12 +143,12 @@ func TestServerShutdown(t *testing.T) {
 	if _, _, err := statSink.NextReader(); err == nil {
 		t.Fatal("Connection not closed")
 	} else {
-		err, ok := err.(*websocket.CloseError)
-		if !ok {
+		var errClose *websocket.CloseError
+		if !errors.As(err, &errClose) {
 			t.Fatal("CloseError not received")
 		}
-		if err.Code != 1012 {
-			t.Fatalf("CloseError with unexpected close code %d received", err.Code)
+		if errClose.Code != 1012 {
+			t.Fatalf("CloseError with unexpected close code %d received", errClose.Code)
 		}
 	}
 

pkg/http/handler/timeout_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package handler
 
 import (
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -67,7 +68,7 @@ func TestTimeoutWriterErrorsWriteAfterTimeout(t *testing.T) {
 	recorder := httptest.NewRecorder()
 	handler := &timeoutWriter{w: recorder}
 	handler.timeoutAndWriteError("error")
-	if _, err := handler.Write([]byte("hello")); err != http.ErrHandlerTimeout {
+	if _, err := handler.Write([]byte("hello")); !errors.Is(err, http.ErrHandlerTimeout) {
 		t.Errorf("ErrHandlerTimeout got %v, want: %s", err, http.ErrHandlerTimeout)
 	}
 }
@@ -148,7 +149,7 @@ func TestTimeToFirstByteTimeoutHandler(t *testing.T) {
 
 			defer func() {
 				if test.wantPanic {
-					if recovered := recover(); recovered != http.ErrAbortHandler {
+					if recovered := recover(); recovered != http.ErrAbortHandler { //nolint // False positive for errors.Is check
 						t.Errorf("Recover = %v, want: %v", recovered, http.ErrAbortHandler)
 					}
 				}
@@ -167,7 +168,7 @@ func TestTimeToFirstByteTimeoutHandler(t *testing.T) {
 			}
 
 			if test.wantWriteError {
-				if err := <-writeErrors; err != http.ErrHandlerTimeout {
+				if err := <-writeErrors; !errors.Is(err, http.ErrHandlerTimeout) {
 					t.Error("Expected a timeout error, got", err)
 				}
 			}

pkg/queue/breaker_test.go

@@ -18,6 +18,7 @@ package queue
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -277,7 +278,7 @@ func TestSemaphoreRelease(t *testing.T) {
 	if err := sem.release(); err != nil {
 		t.Errorf("release = %v; want: %v", err, nil)
 	}
-	if err := sem.release(); err != ErrRelease {
+	if err := sem.release(); !errors.Is(err, ErrRelease) {
 		t.Errorf("release = %v; want: %v", err, ErrRelease)
 	}
 }

pkg/queue/handler.go

@@ -18,6 +18,7 @@ package queue
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"time"
 
@@ -63,10 +64,9 @@ func ProxyHandler(breaker *Breaker, stats *network.RequestStats, tracingEnabled
 				next.ServeHTTP(w, r)
 			}); err != nil {
 				waitSpan.End()
-				switch err {
-				case context.DeadlineExceeded, ErrRequestQueueFull:
+				if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, ErrRequestQueueFull) {
 					http.Error(w, err.Error(), http.StatusServiceUnavailable)
-				default:
+				} else {
 					// This line is most likely untestable :-).
 					w.WriteHeader(http.StatusInternalServerError)
 				}

pkg/reconciler/accessor/errors.go

@@ -16,7 +16,10 @@ limitations under the License.
 
 package accessor
 
-import "strings"
+import (
+	"errors"
+	"strings"
+)
 
 // Error defines a type of error coming from Accessor.
 type Error struct {
@@ -43,8 +46,8 @@ func (a Error) Error() string {
 
 // IsNotOwned returns true if the error is caused by NotOwnResource.
 func IsNotOwned(err error) bool {
-	accessorError, ok := err.(Error)
-	if !ok {
+	var accessorError Error
+	if !errors.As(err, &accessorError) {
 		return false
 	}
 	return accessorError.errorReason == NotOwnResource

pkg/reconciler/gc/v1/gc.go

@@ -18,6 +18,7 @@ package v1
 
 import (
 	"context"
+	"errors"
 	"sort"
 	"time"
 
@@ -88,7 +89,8 @@ func isRevisionStale(ctx context.Context, rev *v1.Revision, config *v1.Configura
 
 	lastPin, err := rev.GetLastPinned()
 	if err != nil {
-		if err.(v1.LastPinnedParseError).Type != v1.AnnotationParseErrorTypeMissing {
+		var errLastPinned v1.LastPinnedParseError
+		if errors.As(err, &errLastPinned) && errLastPinned.Type != v1.AnnotationParseErrorTypeMissing {
 			logger.Errorw("Failed to determine revision last pinned", zap.Error(err))
 		} else {
 			// Revision was never pinned and its RevisionConditionReady is not true after staleRevisionCreateDelay.

pkg/reconciler/metric/metric.go

@@ -18,6 +18,7 @@ package metric
 
 import (
 	"context"
+	"errors"
 
 	"knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	"knative.dev/serving/pkg/autoscaler/metrics"
@@ -37,10 +38,10 @@ var _ metricreconciler.Interface = (*reconciler)(nil)
 // ReconcileKind implements Interface.ReconcileKind.
 func (r *reconciler) ReconcileKind(ctx context.Context, metric *v1alpha1.Metric) pkgreconciler.Event {
 	if err := r.collector.CreateOrUpdate(metric); err != nil {
-		switch err {
-		case metrics.ErrFailedGetEndpoints:
+		switch {
+		case errors.Is(err, metrics.ErrFailedGetEndpoints):
 			metric.Status.MarkMetricNotReady("NoEndpoints", err.Error())
-		case metrics.ErrDidNotReceiveStat:
+		case errors.Is(err, metrics.ErrDidNotReceiveStat):
 			metric.Status.MarkMetricFailed("DidNotReceiveStat", err.Error())
 		default:
 			metric.Status.MarkMetricFailed("CollectionFailed",