From b3ee15cc9a84f7ad31edd9bcb491fe0a17108efe Mon Sep 17 00:00:00 2001 From: Danny Date: Wed, 25 Apr 2018 11:59:30 +0200 Subject: [PATCH] universal error handling in api pkg by returning errs from http handlers --- pkg/api/auth.go | 20 +++++++----- pkg/api/browsers.go | 4 +-- pkg/api/collect.go | 34 +++++++++---------- pkg/api/http.go | 43 +++++++++++++++++++++++++ pkg/api/languages.go | 4 +-- pkg/api/pageviews.go | 30 ++++++++++------- pkg/api/{api.go => params.go} | 32 +++++++----------- pkg/api/{api_test.go => params_test.go} | 0 pkg/api/referrers.go | 4 +-- pkg/api/screens.go | 4 +-- pkg/api/visitors.go | 12 +++---- 11 files changed, 116 insertions(+), 71 deletions(-) create mode 100644 pkg/api/http.go rename pkg/api/{api.go => params.go} (71%) rename pkg/api/{api_test.go => params_test.go} (100%) diff --git a/pkg/api/auth.go b/pkg/api/auth.go index 6027271..707c696 100644 --- a/pkg/api/auth.go +++ b/pkg/api/auth.go @@ -25,7 +25,7 @@ type login struct { var store = sessions.NewCookieStore([]byte(os.Getenv("ANA_SECRET_KEY"))) // URL: POST /api/session -var LoginHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var LoginHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { // check login creds var l login @@ -36,27 +36,31 @@ var LoginHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) // compare pwd if err != nil || bcrypt.CompareHashAndPassword([]byte(u.HashedPassword), []byte(l.Password)) != nil { w.WriteHeader(http.StatusUnauthorized) - respond(w, envelope{Error: "invalid_credentials"}) - return + return respond(w, envelope{Error: "invalid_credentials"}) } session, _ := store.Get(r, "auth") session.Values["user_id"] = u.ID err = session.Save(r, w) - checkError(err) + if err != nil { + return err + } - respond(w, envelope{Data: true}) + return respond(w, envelope{Data: true}) }) // URL: DELETE /api/session -var LogoutHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var LogoutHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { session, _ := store.Get(r, "auth") if !session.IsNew { session.Options.MaxAge = -1 - session.Save(r, w) + err := session.Save(r, w) + if err != nil { + return err + } } - respond(w, envelope{Data: true}) + return respond(w, envelope{Data: true}) }) /* middleware */ diff --git a/pkg/api/browsers.go b/pkg/api/browsers.go index cbe347a..37f7027 100644 --- a/pkg/api/browsers.go +++ b/pkg/api/browsers.go @@ -6,8 +6,8 @@ import ( ) // URL: /api/browsers -var GetBrowsersHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetBrowsersHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) results := count.Browsers(before, after, getRequestedLimit(r)) - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) }) diff --git a/pkg/api/collect.go b/pkg/api/collect.go index efcfccd..b2e4cb9 100644 --- a/pkg/api/collect.go +++ b/pkg/api/collect.go @@ -11,28 +11,23 @@ import ( "github.com/mssola/user_agent" "github.com/usefathom/fathom/pkg/datastore" "github.com/usefathom/fathom/pkg/models" + + log "github.com/sirupsen/logrus" ) var buffer []*models.Pageview var bufferSize = 250 var timeout = 100 * time.Millisecond -func getRequestIp(r *http.Request) string { - ipAddress := r.RemoteAddr - - headerForwardedFor := r.Header.Get("X-Forwarded-For") - if headerForwardedFor != "" { - ipAddress = headerForwardedFor - } - - return ipAddress -} - func persistPageviews() { if len(buffer) > 0 { err := datastore.SavePageviews(buffer) + if err != nil { + log.Errorf("error saving pageviews: %s", err) + } + + // clear buffer regardless of error... this means data loss, but better than filling the buffer for now buffer = buffer[:0] - checkError(err) } } @@ -55,12 +50,12 @@ func NewCollectHandler() http.Handler { pageviews := make(chan *models.Pageview, 100) go processBuffer(pageviews) - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ua := user_agent.New(r.UserAgent()) + return HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { // abort if this is a bot. + ua := user_agent.New(r.UserAgent()) if ua.Bot() { - return + return nil } q := r.URL.Query() @@ -75,7 +70,9 @@ func NewCollectHandler() http.Handler { } err = datastore.SavePage(page) - checkError(err) + if err != nil { + return err + } } // find or insert visitor. @@ -98,7 +95,9 @@ func NewCollectHandler() http.Handler { visitor.BrowserName, visitor.BrowserVersion = ua.Browser() visitor.BrowserName = parseMajorMinor(visitor.BrowserName) err = datastore.SaveVisitor(visitor) - checkError(err) + if err != nil { + return err + } } pageview := &models.Pageview{ @@ -127,6 +126,7 @@ func NewCollectHandler() http.Handler { // 1x1 px transparent GIF b, _ := base64.StdEncoding.DecodeString("R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7") w.Write(b) + return nil }) } diff --git a/pkg/api/http.go b/pkg/api/http.go new file mode 100644 index 0000000..7a387ec --- /dev/null +++ b/pkg/api/http.go @@ -0,0 +1,43 @@ +package api + +import ( + "encoding/json" + log "github.com/sirupsen/logrus" + "net/http" +) + +// Handler is our custom HTTP handler with error returns +type Handler func(w http.ResponseWriter, r *http.Request) error + +func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if err := h(w, r); err != nil { + HandleError(w, r, err) + } +} + +// HandlerFunc takes a custom Handler func and converts it to http.HandlerFunc +func HandlerFunc(fn Handler) http.HandlerFunc { + return http.HandlerFunc(Handler(fn).ServeHTTP) +} + +// HandleError handles errors +func HandleError(w http.ResponseWriter, r *http.Request, err error) { + log.WithFields(log.Fields{ + "request": r.Method + " " + r.RequestURI, + "error": err, + }).Error("error handling request") + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("false")) +} + +type envelope struct { + Data interface{} + Error interface{} `json:"omitempty"` +} + +func respond(w http.ResponseWriter, d interface{}) error { + w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(d) + return err +} diff --git a/pkg/api/languages.go b/pkg/api/languages.go index 0447b27..e924b41 100644 --- a/pkg/api/languages.go +++ b/pkg/api/languages.go @@ -7,8 +7,8 @@ import ( ) // URL: /api/languages -var GetLanguagesHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetLanguagesHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) results := count.Languages(before, after, getRequestedLimit(r)) - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) }) diff --git a/pkg/api/pageviews.go b/pkg/api/pageviews.go index e22196a..5c55d0c 100644 --- a/pkg/api/pageviews.go +++ b/pkg/api/pageviews.go @@ -15,7 +15,7 @@ type pageviews struct { } // URL: /api/pageviews -var GetPageviewsHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetPageviewsHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) stmt, err := datastore.DB.Prepare(` @@ -30,38 +30,46 @@ var GetPageviewsHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.R GROUP BY p.path, p.hostname ORDER BY count DESC LIMIT ?`) - - checkError(err) + if err != nil { + return err + } defer stmt.Close() rows, err := stmt.Query(before, after, defaultLimit) - checkError(err) + if err != nil { + return err + } defer rows.Close() results := make([]pageviews, 0) for rows.Next() { var p pageviews err = rows.Scan(&p.Hostname, &p.Path, &p.Count, &p.CountUnique) - checkError(err) + if err != nil { + return err + } + results = append(results, p) } err = rows.Err() - checkError(err) + if err != nil { + return err + } - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) }) // URL: /api/pageviews/count -var GetPageviewsCountHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetPageviewsCountHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) result := count.Pageviews(before, after) - respond(w, envelope{Data: result}) + return respond(w, envelope{Data: result}) }) // URL: /api/pageviews/group/day -var GetPageviewsPeriodCountHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetPageviewsPeriodCountHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) results := count.PageviewsPerDay(before, after) - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) }) diff --git a/pkg/api/api.go b/pkg/api/params.go similarity index 71% rename from pkg/api/api.go rename to pkg/api/params.go index 1536e34..8de90e4 100644 --- a/pkg/api/api.go +++ b/pkg/api/params.go @@ -1,8 +1,6 @@ package api import ( - "encoding/json" - "log" "net/http" "strconv" "strings" @@ -12,25 +10,6 @@ import ( const defaultPeriod = 7 const defaultLimit = 10 -type envelope struct { - Data interface{} - Error interface{} -} - -func respond(w http.ResponseWriter, d interface{}) { - w.Header().Set("Content-Type", "application/json") - enc := json.NewEncoder(w) - err := enc.Encode(d) - checkError(err) -} - -// log fatal errors -func checkError(err error) { - if err != nil { - log.Fatal(err) - } -} - func getRequestedLimit(r *http.Request) int { limit, err := strconv.Atoi(r.URL.Query().Get("limit")) if err != nil || limit == 0 { @@ -64,3 +43,14 @@ func parseMajorMinor(v string) string { } return v } + +func getRequestIp(r *http.Request) string { + ipAddress := r.RemoteAddr + + headerForwardedFor := r.Header.Get("X-Forwarded-For") + if headerForwardedFor != "" { + ipAddress = headerForwardedFor + } + + return ipAddress +} diff --git a/pkg/api/api_test.go b/pkg/api/params_test.go similarity index 100% rename from pkg/api/api_test.go rename to pkg/api/params_test.go diff --git a/pkg/api/referrers.go b/pkg/api/referrers.go index f271c97..cbb5f26 100644 --- a/pkg/api/referrers.go +++ b/pkg/api/referrers.go @@ -7,8 +7,8 @@ import ( ) // URL: /api/referrers -var GetReferrersHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetReferrersHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) results := count.Referrers(before, after, getRequestedLimit(r)) - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) }) diff --git a/pkg/api/screens.go b/pkg/api/screens.go index 80fd81c..27d33b6 100644 --- a/pkg/api/screens.go +++ b/pkg/api/screens.go @@ -7,8 +7,8 @@ import ( ) // URL: /api/screen-resolutions -var GetScreenResolutionsHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetScreenResolutionsHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) results := count.Screens(before, after, getRequestedLimit(r)) - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) }) diff --git a/pkg/api/visitors.go b/pkg/api/visitors.go index d2cd452..8bea94f 100644 --- a/pkg/api/visitors.go +++ b/pkg/api/visitors.go @@ -7,21 +7,21 @@ import ( ) // URL: /api/visitors/count -var GetVisitorsCountHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetVisitorsCountHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) result := count.Visitors(before, after) - respond(w, envelope{Data: result}) + return respond(w, envelope{Data: result}) }) // URL: /api/visitors/count/realtime -var GetVisitorsRealtimeCountHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetVisitorsRealtimeCountHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { result := count.RealtimeVisitors() - respond(w, envelope{Data: result}) + return respond(w, envelope{Data: result}) }) // URL: /api/visitors/count/group/:period -var GetVisitorsPeriodCountHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { +var GetVisitorsPeriodCountHandler = HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { before, after := getRequestedPeriods(r) results := count.VisitorsPerDay(before, after) - respond(w, envelope{Data: results}) + return respond(w, envelope{Data: results}) })