Skip to content

Commit

Permalink
all: simplify error naming
Browse files Browse the repository at this point in the history 
Don't call them fooErr. Call most of them just "err". This has several
advantages:

* Error values are overriden, so we can't possibly use the wrong error
  by mistake
* Less names in a given scope
* Simpler, shorter code

Errors should only have special names if they are long-lived, like
goAgainErr in main.go.
  • Loading branch information
@mvdan @buger
mvdan authored and buger committed Feb 6, 2017
1 parent d2c5083 commit 7f24486
Show file tree
Hide file tree
Showing 49 changed files with 475 additions and 550 deletions.
67 changes: 30 additions & 37 deletions api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,7 @@ func handleAddOrUpdate(keyName string, r *http.Request) ([]byte, int) {
if dont_reset == "1" {
suppress_reset = true
}
addUpdateErr := doAddOrUpdate(keyName, newSession, suppress_reset)
if addUpdateErr != nil {
if err := doAddOrUpdate(keyName, newSession, suppress_reset); err != nil {
success = false
responseMessage = createError("Failed to create key, ensure security settings are correct.")
}
Expand Down Expand Up @@ -698,15 +697,14 @@ func HandleAddOrUpdateApi(APIID string, r *http.Request) ([]byte, int) {
}

// unmarshal the object into the file
asByte, mErr := json.MarshalIndent(newDef, "", " ")
if mErr != nil {
log.Error("Marshalling of API Definition failed: ", mErr)
asByte, err := json.MarshalIndent(newDef, "", " ")
if err != nil {
log.Error("Marshalling of API Definition failed: ", err)
return createError("Marshalling failed"), 500
}

wErr := ioutil.WriteFile(defFilePath, asByte, 0644)
if wErr != nil {
log.Error("Failed to create file! - ", wErr)
if err := ioutil.WriteFile(defFilePath, asByte, 0644); err != nil {
log.Error("Failed to create file! - ", err)
success = false
return createError("File object creation failed, write error"), 500
}
Expand Down Expand Up @@ -907,14 +905,14 @@ func handleUpdateHashedKey(keyName string, APIID string, policyId string) ([]byt

// TODO: This is pretty ugly
setKeyName := "apikey-" + keyName
rawSessionData, sessErr := sessStore.GetRawKey(setKeyName)
rawSessionData, err := sessStore.GetRawKey(setKeyName)

if sessErr != nil {
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"key": keyName,
"status": "fail",
"err": sessErr,
"err": err,
}).Error("Failed to update hashed key.")

notFound := APIStatusMessage{"error", "Key not found"}
Expand All @@ -923,13 +921,12 @@ func handleUpdateHashedKey(keyName string, APIID string, policyId string) ([]byt
}

sess := SessionState{}
jsErr := json.Unmarshal([]byte(rawSessionData), &sess)
if jsErr != nil {
if err := json.Unmarshal([]byte(rawSessionData), &sess); err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"key": keyName,
"status": "fail",
"err": jsErr,
"err": err,
}).Error("Failed to update hashed key.")

notFound := APIStatusMessage{"error", "Unmarshalling failed"}
Expand All @@ -941,27 +938,26 @@ func handleUpdateHashedKey(keyName string, APIID string, policyId string) ([]byt
sess.LastUpdated = strconv.Itoa(int(time.Now().Unix()))
sess.ApplyPolicyID = policyId

sessAsJS, encErr := json.Marshal(sess)
if encErr != nil {
sessAsJS, err := json.Marshal(sess)
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"key": keyName,
"status": "fail",
"err": encErr,
"err": err,
}).Error("Failed to update hashed key.")

notFound := APIStatusMessage{"error", "Marshalling failed"}
responseMessage, _ = json.Marshal(&notFound)
return responseMessage, 400
}

setErr := sessStore.SetRawKey(setKeyName, string(sessAsJS), 0)
if setErr != nil {
if err := sessStore.SetRawKey(setKeyName, string(sessAsJS), 0); err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"key": keyName,
"status": "fail",
"err": setErr,
"err": err,
}).Error("Failed to update hashed key.")

notFound := APIStatusMessage{"error", "Could not write key data"}
Expand Down Expand Up @@ -1527,14 +1523,13 @@ func createOauthClient(w http.ResponseWriter, r *http.Request) {
}).Error("Failed to create OAuth client")
}

storeErr := apiSpec.OAuthManager.OsinServer.Storage.SetClient(storageID, &newClient, true)

if storeErr != nil {
err := apiSpec.OAuthManager.OsinServer.Storage.SetClient(storageID, &newClient, true)
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"apiID": newOauthClient.APIID,
"status": "fail",
"err": storeErr,
"err": err,
}).Error("Failed to create OAuth client")
responseMessage = createError("Failure in storing client data.")
}
Expand All @@ -1546,7 +1541,6 @@ func createOauthClient(w http.ResponseWriter, r *http.Request) {
PolicyID: newClient.GetPolicyID(),
}

var err error
responseMessage, err = json.Marshal(&reportableClientData)

if err != nil {
Expand Down Expand Up @@ -1611,14 +1605,14 @@ func invalidateOauthRefresh(w http.ResponseWriter, r *http.Request) {
return
}

storeErr := apiSpec.OAuthManager.OsinServer.Storage.RemoveRefresh(keyCombined)
err := apiSpec.OAuthManager.OsinServer.Storage.RemoveRefresh(keyCombined)

if storeErr != nil {
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"apiID": APIID,
"status": "fail",
"err": storeErr,
"err": err,
}).Error("Failed to invalidate refresh token")

DoJSONWrite(w, 400, createError("Failed to invalidate refresh token"))
Expand Down Expand Up @@ -1856,13 +1850,13 @@ func getOauthClients(APIID string) ([]byte, int) {
return responseMessage, 400
}

clientData, getClientsErr := apiSpec.OAuthManager.OsinServer.Storage.GetClients(filterID, true)
if getClientsErr != nil {
clientData, err := apiSpec.OAuthManager.OsinServer.Storage.GetClients(filterID, true)
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "api",
"apiID": APIID,
"status": "fail",
"err": getClientsErr,
"err": err,
}).Error("Failed to report OAuth client list")

success = false
Expand Down Expand Up @@ -1965,8 +1959,8 @@ func UserRatesCheck() http.HandlerFunc {
returnSession.RateLimit.Rate = userSession.Rate
returnSession.RateLimit.Per = userSession.Per

responseMessage, jsonErr := json.Marshal(returnSession)
if jsonErr != nil {
responseMessage, err := json.Marshal(returnSession)
if err != nil {
code = 405
responseMessage = createError("Failed to encode data")
DoJSONWrite(w, code, responseMessage)
Expand All @@ -1980,17 +1974,16 @@ func UserRatesCheck() http.HandlerFunc {
}

func getIPHelper(r *http.Request) string {
var ip string
if clientIP, _, derr := net.SplitHostPort(r.RemoteAddr); derr == nil {
if clientIP, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
// If we aren't the first proxy retain prior
// X-Forwarded-For information as a comma+space
// separated list and fold multiple headers into one.
if prior, ok := r.Header["X-Forwarded-For"]; ok {
clientIP = strings.Join(prior, ", ") + ", " + clientIP
}
ip = clientIP
return clientIP
}
return ip
return ""
}

func invalidateCacheHandler(w http.ResponseWriter, r *http.Request) {
Expand Down
30 changes: 14 additions & 16 deletions api_definition_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,15 @@ func (a *APIDefinitionLoader) LoadDefinitionsFromDashboardService(endpoint strin
c := &http.Client{
Timeout: 120 * time.Second,
}
response, reqErr := c.Do(newRequest)

if reqErr != nil {
log.Error("Request failed: ", reqErr)
response, err := c.Do(newRequest)
if err != nil {
log.Error("Request failed: ", err)
return &APISpecs
}

retBody, bErr := a.readBody(response)
if bErr != nil {
log.Error("Failed to read body: ", bErr)
retBody, err := a.readBody(response)
if err != nil {
log.Error("Failed to read body: ", err)
return &APISpecs
}

Expand Down Expand Up @@ -299,9 +298,8 @@ func (a *APIDefinitionLoader) LoadDefinitionsFromDashboardService(endpoint strin
}

rawList := make(map[string]interface{})
rawdecErr := json.Unmarshal(retBody, &rawList)
if rawdecErr != nil {
log.Error("Failed to decode body (raw): ", rawdecErr)
if err := json.Unmarshal(retBody, &rawList); err != nil {
log.Error("Failed to decode body (raw): ", err)
return &APISpecs
}

Expand Down Expand Up @@ -555,18 +553,18 @@ func (a *APIDefinitionLoader) compileTransformPathSpec(paths []apidef.TemplateMe
newTransformSpec := TransformSpec{TemplateMeta: stringSpec}

// Load the templates
var templErr error
var err error

switch stringSpec.TemplateData.Mode {
case apidef.UseFile:
log.Debug("-- Using File mode")
newTransformSpec.Template, templErr = a.loadFileTemplate(stringSpec.TemplateData.TemplateSource)
newTransformSpec.Template, err = a.loadFileTemplate(stringSpec.TemplateData.TemplateSource)
case apidef.UseBlob:
log.Debug("-- Blob mode")
newTransformSpec.Template, templErr = a.loadBlobTemplate(stringSpec.TemplateData.TemplateSource)
newTransformSpec.Template, err = a.loadBlobTemplate(stringSpec.TemplateData.TemplateSource)
default:
log.Warning("[Transform Templates] No template mode defined! Found: ", stringSpec.TemplateData.Mode)
templErr = errors.New("No valid template mode defined, must be either 'file' or 'blob'")
err = errors.New("No valid template mode defined, must be either 'file' or 'blob'")
}

if stat == Transformed {
Expand All @@ -575,11 +573,11 @@ func (a *APIDefinitionLoader) compileTransformPathSpec(paths []apidef.TemplateMe
newSpec.TransformResponseAction = newTransformSpec
}

if templErr == nil {
if err == nil {
urlSpec = append(urlSpec, newSpec)
log.Debug("-- Loaded")
} else {
log.Error("Template load failure! Skipping transformation: ", templErr)
log.Error("Template load failure! Skipping transformation: ", err)
}

}
Expand Down
6 changes: 3 additions & 3 deletions api_healthcheck.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ func (h *DefaultHealthChecker) GetApiHealthValues() (HealthCheckValues, error) {
log.Debug("V is: ", string(v.([]byte)))
splitValues := strings.Split(string(v.([]byte)), ".")
if len(splitValues) > 1 {
vInt, cErr := strconv.Atoi(splitValues[1])
if cErr != nil {
log.Error("Couldn't convert tracked latency value to Int, vl is: ", cErr)
vInt, err := strconv.Atoi(splitValues[1])
if err != nil {
log.Error("Couldn't convert tracked latency value to Int, vl is: ", err)
} else {
runningTotal += vInt
}
Expand Down
8 changes: 4 additions & 4 deletions auth_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ func (b *DefaultAuthorisationManager) IsKeyAuthorised(keyName string) (SessionSt
return newSession, false
}

if marshalErr := json.Unmarshal([]byte(jsonKeyVal), &newSession); marshalErr != nil {
if err := json.Unmarshal([]byte(jsonKeyVal), &newSession); err != nil {
log.Error("Couldn't unmarshal session object")
log.Error(marshalErr)
log.Error(err)
return newSession, false
}

Expand Down Expand Up @@ -139,8 +139,8 @@ func (b *DefaultSessionManager) GetSessionDetail(keyName string) (SessionState,
return session, false
}

if marshalErr := json.Unmarshal([]byte(jsonKeyVal), &session); marshalErr != nil {
log.Error("Couldn't unmarshal session object (may be cache miss): ", marshalErr)
if err := json.Unmarshal([]byte(jsonKeyVal), &session); err != nil {
log.Error("Couldn't unmarshal session object (may be cache miss): ", err)
return session, false
}

Expand Down
Loading

0 comments on commit 7f24486

Please sign in to comment.