CBG-4282 silence non-constant format govet error

auth/oidc.go

@@ -302,7 +302,7 @@ func (op *OIDCProvider) InitUserPrefix(ctx context.Context) error {
 // an initial provider metadata discovery and initiates the discovery sync in the background.
 func (op *OIDCProvider) initOIDCClient(ctx context.Context) error {
 	if op == nil {
-		return fmt.Errorf(ErrMsgNilProvider)
+		return errors.New(ErrMsgNilProvider)
 	}
 
 	if op.Issuer == "" {

auth/role.go

@@ -355,9 +355,9 @@ func (role *roleImpl) validate() error {
 
 func (role *roleImpl) UnauthError(message string) error {
 	if role.Name_ == "" {
-		return base.HTTPErrorf(http.StatusUnauthorized, "login required: "+message)
+		return base.HTTPErrorf(http.StatusUnauthorized, "login required: %s", message)
 	}
-	return base.HTTPErrorf(http.StatusForbidden, message)
+	return base.NewHTTPError(http.StatusForbidden, message)
 }
 
 // Returns true if the Role is allowed to access the channel.

base/error.go

@@ -96,8 +96,14 @@ func (err *HTTPError) Error() string {
 	return fmt.Sprintf("%d %s", err.Status, err.Message)
 }
 
+// HTTPErrorf creates an HTTPError with an http status code and a formatted message.
 func HTTPErrorf(status int, format string, args ...interface{}) *HTTPError {
-	return &HTTPError{status, fmt.Sprintf(format, args...)}
+	return NewHTTPError(status, fmt.Sprintf(format, args...))
+}
+
+// NewHTTPError creates an HTTPError with an http status code and a message.
+func NewHTTPError(status int, message string) *HTTPError {
+	return &HTTPError{status, message}
 }
 
 // Attempts to map an error to an HTTP status code and message.

base/logger_audit.go

@@ -146,7 +146,7 @@ func Audit(ctx context.Context, id AuditID, additionalData AuditFields) {
 		return
 	}
 
-	logger.logf(fieldsJSON)
+	logger.log(fieldsJSON)
 	SyncGatewayStats.GlobalStats.AuditStat.NumAuditsLogged.Add(1)
 }
 

base/logger_file.go

@@ -141,7 +141,7 @@ func (l *FileLogger) FlushBufferToLog() {
 	l.buffer.Reset()
 
 	if l.Enabled.IsTrue() {
-		l.logf(logString)
+		l.log(logString)
 	}
 }
 
@@ -188,27 +188,37 @@ func (l *FileLogger) logf(format string, args ...interface{}) {
 	if l == nil {
 		return
 	}
-	doPrintf := len(args) > 0
+	// optimize for the common case of no arguments
+	if len(args) == 0 {
+		l.log(format)
+		return
+	}
 	if l.collateBuffer != nil {
 		l.collateBufferWg.Add(1)
-		if doPrintf {
-			l.collateBuffer <- fmt.Sprintf(format, args...)
-		} else {
-			l.collateBuffer <- format
-		}
+		l.collateBuffer <- fmt.Sprintf(format, args...)
 	} else {
-		if doPrintf {
-			l.logger.Printf(format, args...)
-		} else {
-			l.logger.Print(format)
-		}
+		l.logger.Printf(format, args...)
+	}
+}
+
+// log will put the given message into the collation buffer if it exists,
+// otherwise will log the message directly.
+func (l *FileLogger) log(format string) {
+	if l == nil {
+		return
+	}
+	if l.collateBuffer != nil {
+		l.collateBufferWg.Add(1)
+		l.collateBuffer <- format
+	} else {
+		l.logger.Print(format)
 	}
 }
 
-// conditionalPrintf will log the message if the log level is enabled.
-func (l *FileLogger) conditionalPrintf(logLevel LogLevel, format string, args ...interface{}) {
-	if l.shouldLog(logLevel) {
-		l.logf(format, args...)
+// conditionalPrintf will log the message if the logger is enabled.
+func (l *FileLogger) conditionalPrint(msg string) {
+	if l.shouldLog(LevelNone) {
+		l.log(msg)
 	}
 }
 

base/logging.go

@@ -187,7 +187,7 @@ func LogLevelCtx(ctx context.Context, logLevel LogLevel, logKey LogKey, format s
 // The content passed in is expected to be a JSON dictionary.
 func RecordStats(statsJson string) {
 	// if statsLogger is nil, logf will no-op
-	statsLogger.Load().logf(statsJson)
+	statsLogger.Load().log(statsJson)
 }
 
 // logTo is the "core" logging function. All other logging functions (like Debugf(), WarnfCtx(), etc.) end up here.
@@ -276,11 +276,11 @@ func LogSyncGatewayVersion(ctx context.Context) {
 
 	// Log the startup indicator to ALL log files too.
 	msg = addPrefixes(msg, ctx, LevelNone, KeyNone)
-	errorLogger.Load().conditionalPrintf(LevelNone, msg)
-	warnLogger.Load().conditionalPrintf(LevelNone, msg)
-	infoLogger.Load().conditionalPrintf(LevelNone, msg)
-	debugLogger.Load().conditionalPrintf(LevelNone, msg)
-	traceLogger.Load().conditionalPrintf(LevelNone, msg)
+	errorLogger.Load().conditionalPrint(msg)
+	warnLogger.Load().conditionalPrint(msg)
+	infoLogger.Load().conditionalPrint(msg)
+	debugLogger.Load().conditionalPrint(msg)
+	traceLogger.Load().conditionalPrint(msg)
 }
 
 // addPrefixes will modify the format string to add timestamps, log level, and other common prefixes.

channels/sync_runner.go

@@ -154,7 +154,7 @@ func NewSyncRunner(ctx context.Context, funcSource string, timeout time.Duration
 				if len(call.ArgumentList) > 1 {
 					message = call.Argument(1).String()
 				}
-				runner.output.Rejection = base.HTTPErrorf(int(status), message)
+				runner.output.Rejection = base.NewHTTPError(int(status), message)
 			}
 		}
 		return otto.UndefinedValue()

db/attachment.go

@@ -15,7 +15,6 @@ import (
 	"crypto/sha256"
 	"encoding/base64"
 	"errors"
-	"fmt"
 	"net/http"
 
 	"github.com/couchbase/sync_gateway/base"
@@ -346,7 +345,7 @@ func GetAttachmentVersion(meta map[string]interface{}) (int, bool) {
 func GenerateProofOfAttachment(ctx context.Context, attachmentData []byte) (nonce []byte, proof string, err error) {
 	nonce = make([]byte, 20)
 	if _, err := rand.Read(nonce); err != nil {
-		return nil, "", base.HTTPErrorf(http.StatusInternalServerError, fmt.Sprintf("Failed to generate random data: %s", err))
+		return nil, "", base.HTTPErrorf(http.StatusInternalServerError, "Failed to generate random data: %s", err)
 	}
 	proof = ProveAttachment(ctx, attachmentData, nonce)
 	base.TracefCtx(ctx, base.KeyCRUD, "Generated nonce %v and proof %q for attachment: %v", nonce, proof, attachmentData)

db/blip_handler.go

@@ -137,7 +137,7 @@ func (bh *blipHandler) refreshUser() error {
 			// Refresh the BlipSyncContext database
 			err := bc.blipContextDb.ReloadUser(bh.loggingCtx)
 			if err != nil {
-				return base.HTTPErrorf(CBLReconnectErrorCode, fmt.Sprintf("%s", err))
+				return base.NewHTTPError(CBLReconnectErrorCode, err.Error())
 			}
 			newUser := bc.blipContextDb.User()
 			newUser.InitializeRoles()
@@ -191,7 +191,7 @@ func collectionBlipHandler(next blipHandlerFunc) blipHandlerFunc {
 		bh.collectionIdx = &collectionIndex
 		bh.collectionCtx, err = bh.collections.get(&collectionIndex)
 		if err != nil {
-			return base.HTTPErrorf(http.StatusBadRequest, fmt.Sprintf("%s", err))
+			return base.HTTPErrorf(http.StatusBadRequest, "%s", err)
 		}
 		bh.collection = &DatabaseCollectionWithUser{
 			DatabaseCollection: bh.collectionCtx.dbCollection,
@@ -221,7 +221,7 @@ func (bh *blipHandler) handleGetCheckpoint(rq *blip.Message) error {
 		return err
 	}
 	if value == nil {
-		return base.HTTPErrorf(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		return base.NewHTTPError(http.StatusNotFound, http.StatusText(http.StatusNotFound))
 	}
 	response.Properties[GetCheckpointResponseRev] = value[BodyRev].(string)
 	delete(value, BodyRev)
@@ -1293,7 +1293,7 @@ func (bh *blipHandler) handleProveAttachment(rq *blip.Message) error {
 		if base.IsDocNotFoundError(err) {
 			return ErrAttachmentNotFound
 		}
-		return base.HTTPErrorf(http.StatusInternalServerError, fmt.Sprintf("Error getting client attachment: %v", err))
+		return base.HTTPErrorf(http.StatusInternalServerError, "Error getting client attachment: %v", err)
 	}
 
 	proof := ProveAttachment(bh.loggingCtx, attData, nonce)

db/blip_handler_collections.go

@@ -82,9 +82,9 @@ func (bh *blipHandler) handleGetCollections(rq *blip.Message) error {
 				checkpoints[i] = Body{}
 				collectionContexts[i] = newBlipSyncCollectionContext(bh.loggingCtx, collection)
 			} else {
-				errMsg := fmt.Sprintf("Unable to fetch client checkpoint %q for collection %s: %s", key, scopeAndCollection, err)
+				errMsg := fmt.Sprintf("Unable to fetch client checkpoint %q for collection %s: %s", key, base.MD(scopeAndCollection).Redact(), err)
 				base.WarnfCtx(bh.loggingCtx, errMsg)
-				return base.HTTPErrorf(http.StatusServiceUnavailable, errMsg)
+				return base.NewHTTPError(http.StatusServiceUnavailable, errMsg)
 			}
 			continue
 		}

db/database.go

@@ -825,7 +825,7 @@ func (dc *DatabaseContext) TakeDbOffline(ctx context.Context, reason string) err
 		}
 
 		base.InfofCtx(ctx, base.KeyCRUD, msg)
-		return base.HTTPErrorf(http.StatusServiceUnavailable, msg)
+		return base.NewHTTPError(http.StatusServiceUnavailable, msg)
 	}
 }
 

db/database_test.go

@@ -3063,7 +3063,7 @@ func TestGetDatabaseCollectionWithUserDefaultCollection(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
-		t.Run(fmt.Sprintf(testCase.name), func(t *testing.T) {
+		t.Run(testCase.name, func(t *testing.T) {
 
 			ctx := base.TestCtx(t)
 			dbCtx, err := NewDatabaseContext(ctx, "db", bucket.NoCloseClone(), false, testCase.options)

db/revision.go

@@ -472,11 +472,11 @@ func GetStringArrayProperty(body map[string]interface{}, property string) ([]str
 		for i := 0; i < len(items); i++ {
 			strings[i], ok = items[i].(string)
 			if !ok {
-				return nil, base.HTTPErrorf(http.StatusBadRequest, property+" must be a string array")
+				return nil, base.HTTPErrorf(http.StatusBadRequest, "%s must be a string array", property)
 			}
 		}
 		return strings, nil
 	} else {
-		return nil, base.HTTPErrorf(http.StatusBadRequest, property+" must be a string array")
+		return nil, base.HTTPErrorf(http.StatusBadRequest, "%s must be a string array", property)
 	}
 }

db/validation.go

@@ -59,7 +59,7 @@ func validateImportBody(body Body) error {
 	disallowed := []string{BodyId, BodyRev, BodyExpiry, BodyRevisions}
 	for _, prop := range disallowed {
 		if _, ok := body[prop]; ok {
-			return base.HTTPErrorf(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property therefore cannot be imported")
+			return base.NewHTTPError(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property therefore cannot be imported")
 		}
 	}
 	// TODO: Validate attachment data to ensure user is not setting invalid attachments
@@ -76,7 +76,7 @@ func validateBlipBody(ctx context.Context, rawBody []byte, doc *Document) error
 		// Only unmarshal if raw body contains the disallowed property
 		if bytes.Contains(rawBody, []byte(`"`+prop+`"`)) {
 			if _, ok := doc.Body(ctx)[prop]; ok {
-				return base.HTTPErrorf(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property")
+				return base.NewHTTPError(http.StatusNotFound, "top-level property '"+prop+"' is a reserved internal property")
 			}
 		}
 	}

rest/admin_api.go

@@ -57,12 +57,12 @@ func (h *handler) handleCreateDB() error {
 	config.Name = dbName
 	if h.server.persistentConfig {
 		if err := config.validatePersistentDbConfig(); err != nil {
-			return base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 
 		validateReplications := true
 		if err := config.validate(h.ctx(), validateOIDC, validateReplications); err != nil {
-			return base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 
 		version, err := GenerateDatabaseConfigVersionID(h.ctx(), "", config)
@@ -597,7 +597,7 @@ func (h *handler) handlePutDbConfig() (err error) {
 	validateReplications := true
 	err = dbConfig.validate(h.ctx(), validateOIDC, validateReplications)
 	if err != nil {
-		return base.HTTPErrorf(http.StatusBadRequest, err.Error())
+		return base.NewHTTPError(http.StatusBadRequest, err.Error())
 	}
 
 	if !h.server.persistentConfig {
@@ -606,7 +606,7 @@ func (h *handler) handlePutDbConfig() (err error) {
 		err = updatedDbConfig.validateConfigUpdate(h.ctx(), oldDBConfig,
 			validateOIDC)
 		if err != nil {
-			return base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 
 		dbCreds, _ := h.server.Config.DatabaseCredentials[dbName]
@@ -648,10 +648,10 @@ func (h *handler) handlePutDbConfig() (err error) {
 		}
 
 		if err := dbConfig.validatePersistentDbConfig(); err != nil {
-			return nil, base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return nil, base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 		if err := bucketDbConfig.validateConfigUpdate(h.ctx(), oldBucketDbConfig, validateOIDC); err != nil {
-			return nil, base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return nil, base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 
 		bucketDbConfig.Version, err = GenerateDatabaseConfigVersionID(h.ctx(), bucketDbConfig.Version, &bucketDbConfig.DbConfig)
@@ -1069,7 +1069,7 @@ func (h *handler) handlePutCollectionConfigSync() error {
 
 		validateReplications := false
 		if err := bucketDbConfig.validate(h.ctx(), !h.getBoolQuery(paramDisableOIDCValidation), validateReplications); err != nil {
-			return nil, base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return nil, base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 
 		bucketDbConfig.Version, err = GenerateDatabaseConfigVersionID(h.ctx(), bucketDbConfig.Version, &bucketDbConfig.DbConfig)
@@ -1239,7 +1239,7 @@ func (h *handler) handlePutCollectionConfigImportFilter() error {
 
 		validateReplications := false
 		if err := bucketDbConfig.validate(h.ctx(), !h.getBoolQuery(paramDisableOIDCValidation), validateReplications); err != nil {
-			return nil, base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return nil, base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 
 		bucketDbConfig.Version, err = GenerateDatabaseConfigVersionID(h.ctx(), bucketDbConfig.Version, &bucketDbConfig.DbConfig)
@@ -1470,7 +1470,7 @@ func (h *handler) handleSetLogging() error {
 	var setLogLevel bool
 	if level := h.getQuery("logLevel"); level != "" {
 		if err := newLogLevel.UnmarshalText([]byte(level)); err != nil {
-			return base.HTTPErrorf(http.StatusBadRequest, err.Error())
+			return base.NewHTTPError(http.StatusBadRequest, err.Error())
 		}
 		setLogLevel = true
 	} else if level := h.getIntQuery("level", 0); level != 0 {
@@ -1697,7 +1697,7 @@ func (h *handler) updatePrincipal(name string, isUser bool) error {
 	}
 
 	if err = auth.ValidatePrincipalName(internalName); err != nil {
-		return base.HTTPErrorf(http.StatusBadRequest, err.Error())
+		return base.NewHTTPError(http.StatusBadRequest, err.Error())
 	}
 
 	newInfo.Name = &internalName
@@ -1891,7 +1891,7 @@ func (h *handler) getUsers() error {
 	nameOnly, _ := h.getOptBoolQuery(paramNameOnly, true)
 
 	if limit > 0 && nameOnly {
-		return base.HTTPErrorf(http.StatusBadRequest, fmt.Sprintf("Use of %s only supported when %s=false", paramLimit, paramNameOnly))
+		return base.HTTPErrorf(http.StatusBadRequest, "Use of %s only supported when %s=false", paramLimit, paramNameOnly)
 	}
 
 	var bytes []byte
@@ -1910,7 +1910,7 @@ func (h *handler) getUsers() error {
 		bytes, marshalErr = base.JSONMarshal(users)
 	} else {
 		if h.db.Options.UseViews {
-			return base.HTTPErrorf(http.StatusBadRequest, fmt.Sprintf("Use of %s=false not supported when database has use_views=true", paramNameOnly))
+			return base.HTTPErrorf(http.StatusBadRequest, "Use of %s=false not supported when database has use_views=true", paramNameOnly)
 		}
 		users, err := h.db.GetUsers(h.ctx(), int(limit))
 		if err != nil {

rest/adminapitest/admin_api_test.go

@@ -2851,7 +2851,7 @@ func TestChannelNameSizeWarningDocChannelUpdate(t *testing.T) {
 		version := rt.PutDoc("replaceNewChannel", `{"chan":"`+chanName+`"}`) // init doc
 		before := rt.GetDatabase().DbStats.Database().WarnChannelNameSizeCount.Value()
 		chanName = strings.Repeat("D", channelLength+5)
-		_ = rt.UpdateDoc("replaceNewChannel", version, fmt.Sprintf(`{"chan":"`+chanName+`", "data":"test"}`))
+		_ = rt.UpdateDoc("replaceNewChannel", version, `{"chan":"`+chanName+`", "data":"test"}`)
 		after := rt.GetDatabase().DbStats.Database().WarnChannelNameSizeCount.Value()
 		assert.Equal(t, before+1, after)
 	})

rest/api.go

@@ -10,7 +10,7 @@ package rest
 
 import (
 	"encoding/json"
-	"fmt"
+	"errors"
 	"net/http"
 	httpprof "net/http/pprof"
 	"os"
@@ -236,8 +236,7 @@ func (h *handler) handleFlush() error {
 		// If it's not a walrus bucket, don't allow flush unless the unsupported config is set
 		if !h.db.BucketSpec.IsWalrusBucket() {
 			if !h.db.DatabaseContext.AllowFlushNonCouchbaseBuckets() {
-				msg := "Flush not allowed on Couchbase buckets by default."
-				return fmt.Errorf(msg)
+				return errors.New("Flush not allowed on Couchbase buckets by default.")
 			}
 		}
 

rest/blip_api_crud_test.go

@@ -498,7 +498,7 @@ func TestBlipSubChangesDocIDFilter(t *testing.T) {
 				// Ensure we only receive expected docs
 				_, isExpected := docIDsReceived[docID]
 				if !isExpected {
-					t.Errorf(fmt.Sprintf("Received unexpected docId: %s", docID))
+					t.Errorf("Received unexpected docId: %s", docID)
 				} else {
 					// Add to received set, to ensure we get all expected docs
 					docIDsReceived[docID] = true