changes from review

Author: Half-Shot

errors.go

@@ -1,16 +1,17 @@
 package main
-
-// ErrCodeBadContent is reported when the reports content could not be parsed.
-const ErrCodeBadContent = "RS_BAD_CONTENT";
-// ErrCodeBadHeader is reported when a header was not able to be parsed.
-const ErrCodeBadHeader = "RS_BAD_HEADER";
-// ErrCodeContentTooLarge is reported when the reported content size is too large.
-const ErrCodeContentTooLarge = "RS_CONTENT_TOO_LARGE";
-// ErrCodeDisallowedApp is reported when a report was rejected due to the report being sent from an unsupported
-const ErrCodeDisallowedApp = "RS_DISALLOWED_APP";
-// ErrCodeMethodNotAllowed is reported when you have used the wrong method for an endpoint.
-const ErrCodeMethodNotAllowed = "RS_METHOD_NOT_ALLOWED";
-// ErrCodeRejected is reported when the submission could be understood but was rejected by RejectionConditions.
-const ErrCodeRejected = "RS_REJECTED";
-// ErrCodeUnknown is a catch-all error when the appliation does not have a specific error.
-const ErrCodeUnknown = "RS_UNKNOWN";
+const (
+	// ErrCodeBadContent is reported when the reports content could not be parsed.
+	ErrCodeBadContent = "RS_BAD_CONTENT";
+	// ErrCodeBadHeader is reported when a header was not able to be parsed.
+	ErrCodeBadHeader = "RS_BAD_HEADER";
+	// ErrCodeContentTooLarge is reported when the reported content size is too large.
+	ErrCodeContentTooLarge = "RS_CONTENT_TOO_LARGE";
+	// ErrCodeDisallowedApp is reported when a report was rejected due to the report being sent from an unsupported
+	ErrCodeDisallowedApp = "RS_DISALLOWED_APP";
+	// ErrCodeMethodNotAllowed is reported when you have used the wrong method for an endpoint.
+	ErrCodeMethodNotAllowed = "RS_METHOD_NOT_ALLOWED";
+	// ErrCodeRejected is reported when the submission could be understood but was rejected by RejectionConditions.
+	ErrCodeRejected = "RS_REJECTED";
+	// ErrCodeUnknown is a catch-all error when the appliation does not have a specific error.
+	ErrCodeUnknown = "RS_UNKNOWN";
+)

main.go

@@ -154,7 +154,7 @@ func (c RejectionCondition) matchesUserText(p *payload) bool {
 	return c.UserTextMatch == "" || regexp.MustCompile(c.UserTextMatch).MatchString(p.UserText)
 }
 
-// Returns a rejection reason and error code if the payload should be rejected by this condition, otherwise nil.
+// Returns a rejection reason and error code if the payload should be rejected by this condition, condition; otherwise returns `nil` for both results.
 func (c RejectionCondition) shouldReject(p *payload) (*string, *string) {
 	if c.matchesApp(p) && c.matchesVersion(p) && c.matchesLabel(p) && c.matchesUserText(p) {
 		// RejectionCondition matches all of the conditions: we should reject this submission/
@@ -171,7 +171,7 @@ func (c RejectionCondition) shouldReject(p *payload) (*string, *string) {
 	return nil, nil
 }
 
-// Returns a rejection reason and error code if the payload should be rejected by any condition, otherwise nil.
+// Returns a rejection reason and error code if the payload should be rejected by any condition, condition; otherwise returns `nil` for both results.
 func (c *config) matchesRejectionCondition(p *payload) (*string, *string) {
 	for _, rc := range c.RejectionConditions {
 		reject, code := rc.shouldReject(p)
@@ -355,8 +355,8 @@ func loadConfig(configPath string) (*config, error) {
 	}
 
 	for idx, condition := range cfg.RejectionConditions {
-		if condition.ErrorCode != "" && strings.HasPrefix(condition.ErrorCode, "RS_REJECTED_") == false {
-			return nil, fmt.Errorf("Rejected condition %d was invalid. ErrorCode must be use the namespace RS_REJECTED_", idx);
+		if condition.ErrorCode != "" && !strings.HasPrefix(condition.ErrorCode, "RS_REJECTED_") {
+			return nil, fmt.Errorf("Rejected condition %d was invalid. `errorcode` must be use the namespace RS_REJECTED_", idx);
 		}
 	}
 	return &cfg, nil

submit.go

@@ -168,23 +168,19 @@ type submitResponse struct {
 type submitErrorResponse struct {
 	Error string `json:"error"`
 	ErrorCode string `json:"errcode"`
-}
-
-type submitPolicyErrorResponse struct {
-	Error string `json:"error"`
-	ErrorCode string `json:"errcode"`
 	PolicyURL string `json:"policy_url,omitempty"`
 }
 
 
+
 func (s *submitServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	// if we attempt to return a response without reading the request body,
 	// apache gets upset and returns a 500. Let's try this.
 	defer req.Body.Close()
 	defer io.Copy(ioutil.Discard, req.Body)
 
 	if req.Method != "POST" && req.Method != "OPTIONS" {
-		writeError(w, 405, submitErrorResponse{"Method not allowed. Use POST.", ErrCodeMethodNotAllowed})
+		writeError(w, 405, submitErrorResponse{Error: "Method not allowed. Use POST.", ErrorCode: ErrCodeMethodNotAllowed})
 		return
 	}
 
@@ -210,7 +206,7 @@ func (s *submitServer) handleSubmission(w http.ResponseWriter, req *http.Request
 	reportDir := filepath.Join("bugs", prefix)
 	if err := os.MkdirAll(reportDir, os.ModePerm); err != nil {
 		log.Println("Unable to create report directory", err)
-		writeError(w, 500, submitErrorResponse{"Internal error", ErrCodeUnknown})
+		writeError(w, 500, submitErrorResponse{Error: "Internal error", ErrorCode: ErrCodeUnknown})
 		return
 	}
 
@@ -235,7 +231,7 @@ func (s *submitServer) handleSubmission(w http.ResponseWriter, req *http.Request
 			log.Printf("Unable to remove report dir %s after rejected upload: %v\n",
 				reportDir, err)
 		}
-		writeError(w, 400, submitPolicyErrorResponse{"This server does not accept rageshakes from your application.", ErrCodeDisallowedApp, "https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md"})
+		writeError(w, 400, submitErrorResponse{"This server does not accept rageshakes from your application.", "RS_DISALLOWED_APP", "https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md"})
 		return
 	}
 	rejection, code := s.cfg.matchesRejectionCondition(p)
@@ -246,7 +242,7 @@ func (s *submitServer) handleSubmission(w http.ResponseWriter, req *http.Request
 				reportDir, err)
 		}
 		userErrorText := fmt.Sprintf("This server did not accept the rageshake because it matches a rejection condition: %s.", *rejection)
-		writeError(w, 400, submitPolicyErrorResponse{userErrorText, *code, "https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md"})
+		writeError(w, 400, submitErrorResponse{userErrorText, *code, "https://github.com/matrix-org/rageshake/blob/master/docs/blocked_rageshake.md"})
 		return
 	}
 
@@ -258,7 +254,7 @@ func (s *submitServer) handleSubmission(w http.ResponseWriter, req *http.Request
 	resp, err := s.saveReport(req.Context(), *p, reportDir, listingURL)
 	if err != nil {
 		log.Println("Error handling report submission:", err)
-		http.Error(w, "Internal error", 500)
+		writeError(w, 500, submitErrorResponse{Error: "Could not save report", ErrorCode: ErrCodeUnknown})
 		return
 	}
 
@@ -267,7 +263,7 @@ func (s *submitServer) handleSubmission(w http.ResponseWriter, req *http.Request
 	json.NewEncoder(w).Encode(resp)
 }
 
-func writeError(w http.ResponseWriter, status int, response interface{}) {
+func writeError(w http.ResponseWriter, status int, response submitErrorResponse) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(status)
 	json.NewEncoder(w).Encode(response)
@@ -279,12 +275,12 @@ func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *p
 	length, err := strconv.Atoi(req.Header.Get("Content-Length"))
 	if err != nil {
 		log.Println("Couldn't parse content-length", err)
-		writeError(w, 400, submitErrorResponse{"Bad Content-Length header", ErrCodeBadHeader})
+		writeError(w, 400, submitErrorResponse{Error: "Bad Content-Length header", ErrorCode: ErrCodeBadHeader})
 		return nil
 	}
 	if length > maxPayloadSize {
 		log.Println("Content-length", length, "too large")
-		writeError(w, 413, submitErrorResponse{"Content too large", ErrCodeContentTooLarge})
+		writeError(w, 413, submitErrorResponse{Error: "Content too large", ErrorCode: ErrCodeContentTooLarge})
 		return nil
 	}
 
@@ -295,7 +291,7 @@ func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *p
 			p, err1 := parseMultipartRequest(w, req, reportDir)
 			if err1 != nil {
 				log.Println("Error parsing multipart data:", err1)
-				writeError(w, 400, submitErrorResponse{"Bad multipart data", ErrCodeBadContent})
+				writeError(w, 400, submitErrorResponse{Error: "Bad multipart data", ErrorCode: ErrCodeBadContent})
 				return nil
 			}
 			return p
@@ -305,7 +301,7 @@ func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *p
 	p, err := parseJSONRequest(w, req, reportDir)
 	if err != nil {
 		log.Println("Error parsing JSON body", err)
-		writeError(w, 400, submitErrorResponse{fmt.Sprintf("Could not decode payload: %s", err.Error()), ErrCodeBadContent})
+		writeError(w, 400, submitErrorResponse{Error: fmt.Sprintf("Could not decode payload: %s", err.Error()), ErrorCode: ErrCodeBadContent})
 		return nil
 	}