fix(report): propagate Write errors; shorten query/upload paths (ask 44)

- Return io.WriteString errors from WriteReports via writeReportString
- Split ParseReportQuery into small URL parsing helpers
- Extract serveUploadPut and uploadAuthorized from upload handler closure
- Build report header with strings.Builder

Made-with: Cursor

4 files changed, 144 insertions, 97 deletions

diff --git a/internal/daemon/upload.go b/internal/daemon/upload.go
index be6c6d9..71bffd0 100644
--- a/internal/daemon/upload.go
+++ b/internal/daemon/upload.go
@@ -24,57 +24,66 @@ var uploadKinds = map[string]string{
 
 func uploadHandler(statsDir string, store *authkeys.Store) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Method != http.MethodPut {
-			http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
-			return
-		}
-		host, kind, ok := parseUploadPath(r.URL.Path)
-		if !ok {
-			http.Error(w, "bad path", http.StatusBadRequest)
-			return
-		}
-		ext, ok := uploadKinds[kind]
-		if !ok {
-			http.Error(w, "unknown file kind", http.StatusBadRequest)
-			return
-		}
-		ctx := r.Context()
-		nKeys, err := store.KeyCount(ctx)
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusInternalServerError)
-			return
-		}
-		if nKeys > 0 {
-			tok, ok := parseBearer(r.Header.Get("Authorization"))
-			if !ok || tok == "" {
-				w.Header().Set("WWW-Authenticate", `Bearer realm="upload"`)
-				http.Error(w, "unauthorized", http.StatusUnauthorized)
-				return
-			}
-			valid, err := store.Verify(ctx, host, tok)
-			if err != nil {
-				http.Error(w, err.Error(), http.StatusInternalServerError)
-				return
-			}
-			if !valid {
-				http.Error(w, "forbidden", http.StatusForbidden)
-				return
-			}
-		}
-		rel := host + ext
-		target := filepath.Join(statsDir, rel)
-		if !fileUnderDir(statsDir, target) {
-			http.Error(w, "bad path", http.StatusBadRequest)
-			return
-		}
-		if err := writeUploadBody(target, r.Body); err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
-			return
-		}
-		w.WriteHeader(http.StatusNoContent)
+		serveUploadPut(w, r, statsDir, store)
 	})
 }
 
+func serveUploadPut(w http.ResponseWriter, r *http.Request, statsDir string, store *authkeys.Store) {
+	if r.Method != http.MethodPut {
+		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+		return
+	}
+	host, kind, ok := parseUploadPath(r.URL.Path)
+	if !ok {
+		http.Error(w, "bad path", http.StatusBadRequest)
+		return
+	}
+	ext, ok := uploadKinds[kind]
+	if !ok {
+		http.Error(w, "unknown file kind", http.StatusBadRequest)
+		return
+	}
+	ctx := r.Context()
+	nKeys, err := store.KeyCount(ctx)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	if nKeys > 0 && !uploadAuthorized(ctx, w, store, host, r.Header.Get("Authorization")) {
+		return
+	}
+	rel := host + ext
+	target := filepath.Join(statsDir, rel)
+	if !fileUnderDir(statsDir, target) {
+		http.Error(w, "bad path", http.StatusBadRequest)
+		return
+	}
+	if err := writeUploadBody(target, r.Body); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+	w.WriteHeader(http.StatusNoContent)
+}
+
+func uploadAuthorized(ctx context.Context, w http.ResponseWriter, store *authkeys.Store, host, authz string) bool {
+	tok, ok := parseBearer(authz)
+	if !ok || tok == "" {
+		w.Header().Set("WWW-Authenticate", `Bearer realm="upload"`)
+		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		return false
+	}
+	valid, err := store.Verify(ctx, host, tok)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return false
+	}
+	if !valid {
+		http.Error(w, "forbidden", http.StatusForbidden)
+		return false
+	}
+	return true
+}
+
 func parseUploadPath(path string) (host, kind string, ok bool) {
 	const prefix = "/upload/"
 	if !strings.HasPrefix(path, prefix) {
diff --git a/internal/goprecords/report.go b/internal/goprecords/report.go
index 6724c47..11c809a 100644
--- a/internal/goprecords/report.go
+++ b/internal/goprecords/report.go
@@ -62,9 +62,13 @@ func WriteReports(w io.Writer, aggregates *Aggregates, cfg ReportConfig) error {
 			return fmt.Errorf("Category %s only supports: Boots, Uptime, Score", cfg.Category)
 		}
 		if cfg.Category == CategoryHost {
-			io.WriteString(w, NewHostReporter(aggregates, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report())
+			if err := writeReportString(w, NewHostReporter(aggregates, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()); err != nil {
+				return err
+			}
 		} else {
-			io.WriteString(w, NewReporter(aggregates, cfg.Category, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report())
+			if err := writeReportString(w, NewReporter(aggregates, cfg.Category, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()); err != nil {
+				return err
+			}
 		}
 		return nil
 	}
@@ -82,15 +86,26 @@ func WriteReports(w io.Writer, aggregates *Aggregates, cfg ReportConfig) error {
 			continue
 		}
 		if c == CategoryHost {
-			io.WriteString(w, NewHostReporter(aggregates, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report())
+			if err := writeReportString(w, NewHostReporter(aggregates, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()); err != nil {
+				return err
+			}
 		} else {
-			io.WriteString(w, NewReporter(aggregates, c, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report())
+			if err := writeReportString(w, NewReporter(aggregates, c, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()); err != nil {
+				return err
+			}
+		}
+		if err := writeReportString(w, "\n"); err != nil {
+			return err
 		}
-		io.WriteString(w, "\n")
 	}
 	return nil
 }
 
+func writeReportString(w io.Writer, s string) error {
+	_, err := io.WriteString(w, s)
+	return err
+}
+
 type Reporter interface {
 	Report() string
 }
diff --git a/internal/goprecords/report_config.go b/internal/goprecords/report_config.go
index 8cd8f8d..8dc43d8 100644
--- a/internal/goprecords/report_config.go
+++ b/internal/goprecords/report_config.go
@@ -73,51 +73,29 @@ func (rf *ReportFlags) Parse() (ReportConfig, error) {
 // output-format, all, include-kernel, stats-order). It also accepts Category,
 // Metric, and OutputFormat as alternate keys (same values as the CLI).
 func ParseReportQuery(q url.Values) (ReportConfig, error) {
-	catStr := firstQuery(q, "category", "Category")
-	if catStr == "" {
-		catStr = "Host"
-	}
-	cat, err := ParseCategory(catStr)
+	cat, err := parseReportQueryCategory(q)
 	if err != nil {
 		return ReportConfig{}, err
 	}
-	metStr := firstQuery(q, "metric", "Metric")
-	if metStr == "" {
-		metStr = "Uptime"
-	}
-	met, err := ParseMetric(metStr)
+	met, err := parseReportQueryMetric(q)
 	if err != nil {
 		return ReportConfig{}, err
 	}
-	limit := uint(20)
-	if ls := q.Get("limit"); ls != "" {
-		v, err := strconv.ParseUint(ls, 10, 32)
-		if err != nil {
-			return ReportConfig{}, fmt.Errorf("invalid limit %q", ls)
-		}
-		limit = uint(v)
-	}
-	outStr := firstQuery(q, "output-format", "OutputFormat")
-	if outStr == "" {
-		outStr = "Plaintext"
+	limit, err := parseReportQueryLimit(q)
+	if err != nil {
+		return ReportConfig{}, err
 	}
-	outFmt, err := ParseOutputFormat(outStr)
+	outFmt, err := parseReportQueryOutputFormat(q)
 	if err != nil {
 		return ReportConfig{}, err
 	}
-	all := false
-	if v := q.Get("all"); v != "" {
-		all, err = parseQueryBool(v)
-		if err != nil {
-			return ReportConfig{}, err
-		}
+	all, err := parseReportQueryOptionalBool(q, "all")
+	if err != nil {
+		return ReportConfig{}, err
 	}
-	includeKernel := false
-	if v := q.Get("include-kernel"); v != "" {
-		includeKernel, err = parseQueryBool(v)
-		if err != nil {
-			return ReportConfig{}, err
-		}
+	includeKernel, err := parseReportQueryOptionalBool(q, "include-kernel")
+	if err != nil {
+		return ReportConfig{}, err
 	}
 	return ReportConfig{
 		Category:      cat,
@@ -130,6 +108,49 @@ func ParseReportQuery(q url.Values) (ReportConfig, error) {
 	}, nil
 }
 
+func parseReportQueryCategory(q url.Values) (Category, error) {
+	s := firstQuery(q, "category", "Category")
+	if s == "" {
+		s = "Host"
+	}
+	return ParseCategory(s)
+}
+
+func parseReportQueryMetric(q url.Values) (Metric, error) {
+	s := firstQuery(q, "metric", "Metric")
+	if s == "" {
+		s = "Uptime"
+	}
+	return ParseMetric(s)
+}
+
+func parseReportQueryLimit(q url.Values) (uint, error) {
+	if ls := q.Get("limit"); ls != "" {
+		v, err := strconv.ParseUint(ls, 10, 32)
+		if err != nil {
+			return 0, fmt.Errorf("invalid limit %q", ls)
+		}
+		return uint(v), nil
+	}
+	return 20, nil
+}
+
+func parseReportQueryOutputFormat(q url.Values) (OutputFormat, error) {
+	s := firstQuery(q, "output-format", "OutputFormat")
+	if s == "" {
+		s = "Plaintext"
+	}
+	return ParseOutputFormat(s)
+}
+
+func parseReportQueryOptionalBool(q url.Values, key string) (bool, error) {
+	v := q.Get(key)
+	if v == "" {
+		return false, nil
+	}
+	return parseQueryBool(v)
+}
+
 func firstQuery(q url.Values, keys ...string) string {
 	for _, k := range keys {
 		if v := q.Get(k); v != "" {
diff --git a/internal/goprecords/report_format.go b/internal/goprecords/report_format.go
index 7ec66b3..0d5e4dd 100644
--- a/internal/goprecords/report_format.go
+++ b/internal/goprecords/report_format.go
@@ -101,29 +101,31 @@ func (r reportBuilder) buildBorder(countW, nameW, valueW, lastKernelW int, hasLa
 }
 
 func (r reportBuilder) buildReportHeader(countW, nameW, valueW, lastKernelW int, hasLastKernel bool, border string, outputFormat OutputFormat) string {
-	var h string
+	var b strings.Builder
 	if outputFormat == FormatMarkdown || outputFormat == FormatGemtext {
-		h = strings.Repeat("#", int(r.headerIndent)) + " "
+		b.WriteString(strings.Repeat("#", int(r.headerIndent)))
+		b.WriteString(" ")
 	}
-	h += fmt.Sprintf("Top %d %s's by %s\n\n", r.limit, r.metric, r.category)
+	b.WriteString(fmt.Sprintf("Top %d %s's by %s\n\n", r.limit, r.metric, r.category))
 	desc := MetricDescription(r.metric)
 	lineLimit := len(border)
 	if outputFormat == FormatPlaintext && lineLimit > 0 && len(desc) > lineLimit-1 {
 		desc = " " + wordWrap(desc, lineLimit-1)
 	}
-	h += desc + "\n\n"
+	b.WriteString(desc)
+	b.WriteString("\n\n")
 	if outputFormat == FormatMarkdown || outputFormat == FormatGemtext {
-		h += "```\n"
+		b.WriteString("```\n")
 	}
-	h += border
+	b.WriteString(border)
 	fmtStr := r.buildFormatStr(countW, nameW, valueW, lastKernelW, hasLastKernel)
 	if hasLastKernel {
-		h += fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String(), "Last Kernel")
+		b.WriteString(fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String(), "Last Kernel"))
 	} else {
-		h += fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String())
+		b.WriteString(fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String()))
 	}
-	h += border
-	return h
+	b.WriteString(border)
+	return b.String()
 }
 
 func (r reportBuilder) buildFormatStr(countW, nameW, valueW, lastKernelW int, hasLastKernel bool) string {