coderd: Handle http response errors in parsePagination

mafredri · mafredri · commit ea2371e9f94d · 2022-05-10T07:29:07.000Z
diff --git a/coderd/pagination.go b/coderd/pagination.go
@@ -1,44 +1,57 @@
 package coderd
 
 import (
+	"fmt"
 	"net/http"
 	"strconv"
 
 	"github.com/google/uuid"
-	"golang.org/x/xerrors"
 
+	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/codersdk"
 )
 
-func parsePagination(r *http.Request) (p codersdk.Pagination, err error) {
+// parsePagination extracts pagination query params from the http request.
+// If an error is encountered, the error is written to w and ok is set to false.
+func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Pagination, ok bool) {
 	var (
 		afterID = uuid.Nil
 		limit   = -1 // Default to no limit and return all results.
 		offset  = 0
 	)
 
+	var err error
 	if s := r.URL.Query().Get("after_id"); s != "" {
 		afterID, err = uuid.Parse(r.URL.Query().Get("after_id"))
 		if err != nil {
-			return p, xerrors.Errorf("after_id must be a valid uuid: %w", err.Error())
+			httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
+				Message: fmt.Sprintf("after_id must be a valid uuid: %s", err.Error()),
+			})
+			return p, false
 		}
 	}
 	if s := r.URL.Query().Get("limit"); s != "" {
 		limit, err = strconv.Atoi(s)
 		if err != nil {
-			return p, xerrors.Errorf("limit must be an integer: %w", err.Error())
+			httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
+				Message: fmt.Sprintf("limit must be an integer: %s", err.Error()),
+			})
+			return p, false
 		}
 	}
 	if s := r.URL.Query().Get("offset"); s != "" {
 		offset, err = strconv.Atoi(s)
 		if err != nil {
-			return p, xerrors.Errorf("offset must be an integer: %w", err.Error())
+			httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
+				Message: fmt.Sprintf("offset must be an integer: %s", err.Error()),
+			})
+			return p, false
 		}
 	}
 
 	return codersdk.Pagination{
 		AfterID: afterID,
 		Limit:   limit,
 		Offset:  offset,
-	}, nil
+	}, true
 }
diff --git a/coderd/templates.go b/coderd/templates.go
@@ -75,9 +75,8 @@ func (api *api) deleteTemplate(rw http.ResponseWriter, r *http.Request) {
 func (api *api) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Request) {
 	template := httpmw.TemplateParam(r)
 
-	paginationParams, err := parsePagination(r)
-	if err != nil {
-		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{Message: fmt.Sprintf("parse pagination request: %s", err.Error())})
+	paginationParams, ok := parsePagination(rw, r)
+	if !ok {
 		return
 	}
 
diff --git a/coderd/users.go b/coderd/users.go
@@ -109,9 +109,8 @@ func (api *api) users(rw http.ResponseWriter, r *http.Request) {
 		statusFilter = r.URL.Query().Get("status")
 	)
 
-	paginationParams, err := parsePagination(r)
-	if err != nil {
-		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{Message: fmt.Sprintf("parse pagination request: %s", err.Error())})
+	paginationParams, ok := parsePagination(rw, r)
+	if !ok {
 		return
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -109,9 +109,8 @@ func (api api) users(rw http.ResponseWriter, r http.Request) {`
`109`	`109`	`statusFilter = r.URL.Query().Get("status")`
`110`	`110`	`)`
`111`	`111`
`112`		`- paginationParams, err := parsePagination(r)`
`113`		`- if err != nil {`
`114`		`- httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{Message: fmt.Sprintf("parse pagination request: %s", err.Error())})`
	`112`	`+ paginationParams, ok := parsePagination(rw, r)`
	`113`	`+ if !ok {`
`115`	`114`	`return`
`116`	`115`	`}`
`117`	`116`