Skip to content

Commit

Permalink
chore: Use proper error types in server response
Browse files Browse the repository at this point in the history 
* Style improvs

Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
  • Loading branch information
@mahendrapaipuri
mahendrapaipuri committed Jan 9, 2024
1 parent 28f38ef commit ff6e9b4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 35 deletions.
50 changes: 21 additions & 29 deletions pkg/jobstats/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ func NewJobstatsServer(c *Config) (*JobstatsServer, func(), error) {

// Open DB connection
var err error
dbConn, err = sql.Open("sqlite3", c.DBConfig.JobstatsDBPath)
if err != nil {
if dbConn, err = sql.Open("sqlite3", c.DBConfig.JobstatsDBPath); err != nil {
return nil, func() {}, err
}
return server, func() {}, nil
Expand Down Expand Up @@ -188,13 +187,12 @@ func (s *JobstatsServer) accounts(w http.ResponseWriter, r *http.Request) {
response = base.AccountsResponse{
Response: base.Response{
Status: "error",
ErrorType: "User Error",
ErrorType: "user_error",
Error: "No user identified",
},
Data: []base.Account{},
}
err := json.NewEncoder(w).Encode(&response)
if err != nil {
if err := json.NewEncoder(w).Encode(&response); err != nil {
level.Error(s.logger).Log("msg", "Failed to encode response", "err", err)
w.Write([]byte("KO"))
}
Expand All @@ -209,13 +207,12 @@ func (s *JobstatsServer) accounts(w http.ResponseWriter, r *http.Request) {
response = base.AccountsResponse{
Response: base.Response{
Status: "error",
ErrorType: "Internal server error",
ErrorType: "data_error",
Error: "Failed to fetch user accounts",
},
Data: []base.Account{},
}
err = json.NewEncoder(w).Encode(&response)
if err != nil {
if err = json.NewEncoder(w).Encode(&response); err != nil {
level.Error(s.logger).Log("msg", "Failed to encode response", "err", err)
w.Write([]byte("KO"))
}
Expand All @@ -230,25 +227,23 @@ func (s *JobstatsServer) accounts(w http.ResponseWriter, r *http.Request) {
},
Data: accounts,
}
err = json.NewEncoder(w).Encode(&response)
if err != nil {
if err = json.NewEncoder(w).Encode(&response); err != nil {
level.Error(s.logger).Log("msg", "Failed to encode response", "err", err)
w.Write([]byte("KO"))
}
}

// Return error response for jobs with setting errorString and errorType in response
func (s *JobstatsServer) jobsErrorResponse(errorString string, errorType string, w http.ResponseWriter) {
func (s *JobstatsServer) jobsErrorResponse(errorType string, errorString string, w http.ResponseWriter) {
response := base.JobsResponse{
Response: base.Response{
Status: "error",
ErrorType: errorString,
Error: errorType,
ErrorType: errorType,
Error: errorString,
},
Data: []base.BatchJob{},
}
err := json.NewEncoder(w).Encode(&response)
if err != nil {
if err := json.NewEncoder(w).Encode(&response); err != nil {
level.Error(s.logger).Log("msg", "Failed to encode response", "err", err)
w.Write([]byte("KO"))
}
Expand All @@ -270,7 +265,7 @@ func (s *JobstatsServer) jobs(w http.ResponseWriter, r *http.Request) {
// If no user found, return empty response
if loggedUser == "" {
w.WriteHeader(http.StatusUnauthorized)
s.jobsErrorResponse("User Error", "No user identified", w)
s.jobsErrorResponse("user_error", "No user identified", w)
return
}

Expand Down Expand Up @@ -325,7 +320,7 @@ func (s *JobstatsServer) jobs(w http.ResponseWriter, r *http.Request) {
if ts, err := strconv.Atoi(f); err != nil {
level.Error(s.logger).Log("msg", "Failed to parse from timestamp", "from", f, "err", err)
w.WriteHeader(http.StatusBadRequest)
s.jobsErrorResponse("Internal server error", "Malformed 'from' timestamp", w)
s.jobsErrorResponse("data_error", "Malformed 'from' timestamp", w)
return
} else {
fromTime = time.Unix(int64(ts), 0)
Expand All @@ -339,7 +334,7 @@ func (s *JobstatsServer) jobs(w http.ResponseWriter, r *http.Request) {
if ts, err := strconv.Atoi(t); err != nil {
level.Error(s.logger).Log("msg", "Failed to parse to timestamp", "to", t, "err", err)
w.WriteHeader(http.StatusBadRequest)
s.jobsErrorResponse("Internal server error", "Malformed 'to' timestamp", w)
s.jobsErrorResponse("data_error", "Malformed 'to' timestamp", w)
return
} else {
toTime = time.Unix(int64(ts), 0)
Expand All @@ -356,7 +351,7 @@ func (s *JobstatsServer) jobs(w http.ResponseWriter, r *http.Request) {
"queryWindow", toTime.Sub(fromTime).String(),
)
w.WriteHeader(http.StatusBadRequest)
s.jobsErrorResponse("Internal server error", "Maximum query window exceeded", w)
s.jobsErrorResponse("data_error", "Maximum query window exceeded", w)
return
}

Expand All @@ -373,7 +368,7 @@ queryJobs:
if err != nil {
level.Error(s.logger).Log("msg", "Failed to fetch jobs", "loggedUser", loggedUser, "err", err)
w.WriteHeader(http.StatusInternalServerError)
s.jobsErrorResponse("Internal server error", "Failed to fetch user jobs", w)
s.jobsErrorResponse("data_error", "Failed to fetch user jobs", w)
return
}

Expand All @@ -385,8 +380,7 @@ queryJobs:
},
Data: jobs,
}
err = json.NewEncoder(w).Encode(&response)
if err != nil {
if err = json.NewEncoder(w).Encode(&response); err != nil {
level.Error(s.logger).Log("msg", "Failed to encode response", "err", err)
w.Write([]byte("KO"))
}
Expand Down Expand Up @@ -459,7 +453,7 @@ func fetchJobs(
queryStmt, err := dbConn.Prepare(strings.Replace(queryString, "*", strings.Join(base.BatchJobFieldNames, ","), 1))
if err != nil {
level.Error(logger).Log(
"msg", "Failed to prepare query SQL statement for jobs", "query", queryString,
"msg", "Failed to prepare query SQL statement for jobs query", "query", queryString,
"queryParams", strings.Join(queryParams, ","), "err", err,
)
return nil, err
Expand All @@ -473,17 +467,16 @@ func fetchJobs(
}

// First make a query to get number of rows that will be returned by query
_ = countStmt.QueryRow(qParams...).Scan(&numJobs)
if err != nil {
level.Error(logger).Log("msg", "Failed to execute count SQL statement for jobs query",
if err = countStmt.QueryRow(qParams...).Scan(&numJobs); err != nil {
level.Error(logger).Log("msg", "Failed to get number of jobs",
"query", queryString, "queryParams", strings.Join(queryParams, ","), "err", err,
)
return nil, err
}

rows, err := queryStmt.Query(qParams...)
if err != nil {
level.Error(logger).Log("msg", "Failed to execute query SQL statement for jobs query",
level.Error(logger).Log("msg", "Failed to get jobs",
"query", queryString, "queryParams", strings.Join(queryParams, ","), "err", err,
)
return nil, err
Expand Down Expand Up @@ -568,8 +561,7 @@ func fetchJobs(

// Ping DB for connection test
func getDBStatus(logger log.Logger) bool {
err := dbConn.Ping()
if err != nil {
if err := dbConn.Ping(); err != nil {
level.Error(logger).Log("msg", "DB Ping failed", "err", err)
return false
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/jobstats/server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func TestAccountsHandlerNoUserHeader(t *testing.T) {
if response.Status != "error" {
t.Errorf("expected error status got %v", response.Status)
}
if response.ErrorType != "User Error" {
t.Errorf("expected User Error type got %v", response.ErrorType)
if response.ErrorType != "user_error" {
t.Errorf("expected user_error type got %v", response.ErrorType)
}
if !reflect.DeepEqual(response.Data, []base.Account{}) {
t.Errorf("expected empty data got %v", response.Data)
Expand Down Expand Up @@ -126,8 +126,8 @@ func TestJobsHandlerNoUserHeader(t *testing.T) {
if response.Status != "error" {
t.Errorf("expected error status got %v", response.Status)
}
if response.ErrorType != "User Error" {
t.Errorf("expected User Error type got %v", response.ErrorType)
if response.ErrorType != "user_error" {
t.Errorf("expected user_error type got %v", response.ErrorType)
}
if !reflect.DeepEqual(response.Data, []base.BatchJob{}) {
t.Errorf("expected empty data got %v", response.Data)
Expand Down Expand Up @@ -238,8 +238,8 @@ func TestJobsHandlerWithMalformedQueryParams(t *testing.T) {
if response.Status != "error" {
t.Errorf("expected error status got %v", response.Status)
}
if response.ErrorType != "Internal server error" {
t.Errorf("expected Internal server error type got %v", response.ErrorType)
if response.ErrorType != "data_error" {
t.Errorf("expected data_error type got %v", response.ErrorType)
}
if !reflect.DeepEqual(response.Data, []base.BatchJob{}) {
t.Errorf("expected empty data got %v", response.Data)
Expand Down

0 comments on commit ff6e9b4

Please sign in to comment.