Fixes to /healthz response (v2.10) (#4467)

Follow up from #4437 content-type fix for v2.9.22, some fixes to the
response from `/healthz` for dev:

- In #[3326](#4097) it was
changed to return 500 status when before we used to return 503 so this
changes it back.
- Also as part of #3326 we started to return `status_code` in the
healthz response (e.g `{"status":"ok","status_code":200}`) so this
removes it for http responses just relying on the http header.

server/monitor.go

@@ -2339,7 +2339,6 @@ func ResponseHandler(w http.ResponseWriter, r *http.Request, data []byte) {
 func handleResponse(code int, w http.ResponseWriter, r *http.Request, data []byte) {
 	// Get callback from request
 	callback := r.URL.Query().Get("callback")
-	// If callback is not empty then
 	if callback != "" {
 		// Response for JSONP
 		w.Header().Set("Content-Type", "application/javascript")
@@ -3198,11 +3197,13 @@ func (s *Server) HandleHealthz(w http.ResponseWriter, r *http.Request) {
 	})
 
 	code := http.StatusOK
-
 	if hs.Error != _EMPTY_ {
 		s.Warnf("Healthcheck failed: %q", hs.Error)
-		code = http.StatusServiceUnavailable
+		code = hs.StatusCode
 	}
+	// Remove StatusCode from JSON representation when responding via HTTP
+	// since this is already in the response.
+	hs.StatusCode = 0
 	b, err := json.Marshal(hs)
 	if err != nil {
 		s.Errorf("Error marshaling response to /healthz request: %v", err)
@@ -3232,7 +3233,7 @@ func (s *Server) healthz(opts *HealthzOptions) *HealthStatus {
 		// if no specific status code was set, set it based on the presence of errors
 		if health.StatusCode == 0 {
 			if health.Error != "" || len(health.Errors) != 0 {
-				health.StatusCode = http.StatusInternalServerError
+				health.StatusCode = http.StatusServiceUnavailable
 			} else {
 				health.StatusCode = http.StatusOK
 			}

server/monitor_test.go

@@ -147,28 +147,29 @@ var (
 )
 
 func readBodyEx(t *testing.T, url string, status int, content string) []byte {
+	t.Helper()
 	resp, err := http.Get(url)
 	if err != nil {
-		stackFatalf(t, "Expected no error: Got %v\n", err)
+		t.Fatalf("Expected no error: Got %v\n", err)
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != status {
-		stackFatalf(t, "Expected a %d response, got %d\n", status, resp.StatusCode)
+		t.Fatalf("Expected a %d response, got %d\n", status, resp.StatusCode)
 	}
 	ct := resp.Header.Get("Content-Type")
 	if ct != content {
-		stackFatalf(t, "Expected %q content-type, got %q\n", content, ct)
+		t.Fatalf("Expected %q content-type, got %q\n", content, ct)
 	}
 	// Check the CORS header for "application/json" requests only.
 	if ct == appJSONContent {
 		acao := resp.Header.Get("Access-Control-Allow-Origin")
 		if acao != "*" {
-			stackFatalf(t, "Expected with %q Content-Type an Access-Control-Allow-Origin header with value %q, got %q\n", appJSONContent, "*", acao)
+			t.Fatalf("Expected with %q Content-Type an Access-Control-Allow-Origin header with value %q, got %q\n", appJSONContent, "*", acao)
 		}
 	}
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		stackFatalf(t, "Got an error reading the body: %v\n", err)
+		t.Fatalf("Got an error reading the body: %v\n", err)
 	}
 	return body
 }
@@ -2302,7 +2303,7 @@ func TestClusterEmptyWhenNotDefined(t *testing.T) {
 	body := readBody(t, fmt.Sprintf("http://127.0.0.1:%d/varz", s.MonitorAddr().Port))
 	var v map[string]interface{}
 	if err := json.Unmarshal(body, &v); err != nil {
-		stackFatalf(t, "Got an error unmarshalling the body: %v\n", err)
+		t.Fatalf("Got an error unmarshalling the body: %v\n", err)
 	}
 	// Cluster can empty, or be defined but that needs to be empty.
 	c, ok := v["cluster"]
@@ -5112,7 +5113,7 @@ func TestHealthzStatusError(t *testing.T) {
 	// Note: Private field access, taking advantage of having the tests in the same package.
 	s.listener = nil
 
-	checkHealthzEndpoint(t, s.MonitorAddr().String(), http.StatusServiceUnavailable, "error")
+	checkHealthzEndpoint(t, s.MonitorAddr().String(), http.StatusInternalServerError, "error")
 }
 
 func TestHealthzStatusUnavailable(t *testing.T) {

server/sublist_test.go

@@ -29,7 +29,6 @@ import (
 	"github.com/nats-io/nuid"
 )
 
-// FIXME(dlc) - this is also used by monitor_test. Not needed with t.Helper.
 func stackFatalf(t *testing.T, f string, args ...interface{}) {
 	lines := make([]string, 0, 32)
 	msg := fmt.Sprintf(f, args...)