TO: log a warning instead of ignoring write errors (#5984)

CHANGELOG.md

@@ -55,6 +55,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#5690](https://github.com/apache/trafficcontrol/issues/5690) - Fixed github action for added/modified db migration file.
 - [#2471](https://github.com/apache/trafficcontrol/issues/2471) - A PR check to ensure added db migration file is the latest.
 - [#5609](https://github.com/apache/trafficcontrol/issues/5609) - Fixed GET /servercheck filter for an extra query param.
+- [#5954](https://github.com/apache/trafficcontrol/issues/5954) - Traffic Ops HTTP response write errors are ignored
 - [#5288](https://github.com/apache/trafficcontrol/issues/5288) - Fixed the ability to create and update a server with MTU value >= 1280.
 - [#5284](https://github.com/apache/trafficcontrol/issues/5284) - Fixed error message when creating a server with non-existent profile
 - Fixed a NullPointerException in TR when a client passes a null SNI hostname in a TLS request

traffic_ops/traffic_ops_golang/api/api.go

@@ -112,6 +112,15 @@ func GoneHandler(w http.ResponseWriter, r *http.Request) {
 	HandleErr(w, r, nil, http.StatusGone, err, nil)
 }
 
+// WriteAndLogErr writes the response and logs a warning if an error occurs. This should be used in favor of simply
+// calling w.Write() so that errors are properly logged for troubleshooting.
+func WriteAndLogErr(w http.ResponseWriter, r *http.Request, bts []byte) {
+	if b, err := w.Write(bts); err != nil {
+		reqID, _ := getReqID(r.Context())
+		log.Warnf("failed to write response (method = %s, URL = %s, request ID = %d, remote addr = %s, bytes written = %d): %v", r.Method, r.URL.String(), reqID, r.RemoteAddr, b, err)
+	}
+}
+
 // WriteResp takes any object, serializes it as JSON, and writes that to w. Any errors are logged and written to w via tc.GetHandleErrorsFunc.
 // This is a helper for the common case; not using this in unusual cases is perfectly acceptable.
 func WriteResp(w http.ResponseWriter, r *http.Request, v interface{}) {
@@ -127,14 +136,14 @@ func WriteRespRaw(w http.ResponseWriter, r *http.Request, v interface{}) {
 	}
 	setRespWritten(r)
 
-	bts, err := json.Marshal(v)
+	respBts, err := json.Marshal(v)
 	if err != nil {
 		log.Errorf("marshalling JSON (raw) for %T: %v", v, err)
 		tc.GetHandleErrorsFunc(w, r)(http.StatusInternalServerError, errors.New(http.StatusText(http.StatusInternalServerError)))
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append(bts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // WriteRespWithSummary writes a JSON-encoded representation of an arbitrary
@@ -166,7 +175,7 @@ func WriteRespVals(w http.ResponseWriter, r *http.Request, v interface{}, vals m
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(append(respBts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // WriteIMSHitResp writes a response to 'w' for an IMS request "hit", using the
@@ -259,11 +268,11 @@ func handleSimpleErr(w http.ResponseWriter, r *http.Request, statusCode int, use
 	respBts, err := json.Marshal(tc.CreateErrorAlerts(userErr))
 	if err != nil {
 		log.Errorln("marshalling error: " + err.Error())
-		w.Write(append([]byte(http.StatusText(http.StatusInternalServerError)), '\n'))
+		WriteAndLogErr(w, r, append([]byte(http.StatusText(http.StatusInternalServerError)), '\n'))
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append(respBts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // RespWriter is a helper to allow a one-line response, for endpoints with a function that returns the object that needs to be written and an error.
@@ -306,7 +315,7 @@ func WriteRespAlert(w http.ResponseWriter, r *http.Request, level tc.AlertLevel,
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append(respBts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // WriteRespAlertNotFound creates an alert indicating that the resource was not found and writes that to w.
@@ -325,7 +334,7 @@ func WriteRespAlertNotFound(w http.ResponseWriter, r *http.Request) {
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 	w.WriteHeader(http.StatusNotFound)
-	w.Write(append(respBts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // WriteRespAlertObj Writes the given alert, and the given response object.
@@ -350,7 +359,7 @@ func WriteRespAlertObj(w http.ResponseWriter, r *http.Request, level tc.AlertLev
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	_, _ = w.Write(append(respBts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 func WriteAlerts(w http.ResponseWriter, r *http.Request, code int, alerts tc.Alerts) {
@@ -363,12 +372,12 @@ func WriteAlerts(w http.ResponseWriter, r *http.Request, code int, alerts tc.Ale
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 	w.WriteHeader(code)
 	if alerts.HasAlerts() {
-		resp, err := json.Marshal(alerts)
+		respBts, err := json.Marshal(alerts)
 		if err != nil {
 			handleSimpleErr(w, r, http.StatusInternalServerError, nil, fmt.Errorf("marshalling JSON: %v", err))
 			return
 		}
-		_, _ = w.Write(append(resp, '\n'))
+		WriteAndLogErr(w, r, append(respBts, '\n'))
 	}
 }
 
@@ -398,7 +407,7 @@ func WriteAlertsObj(w http.ResponseWriter, r *http.Request, code int, alerts tc.
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 	w.WriteHeader(code)
-	w.Write(append(respBts, '\n'))
+	WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // IntParams parses integer parameters, and returns map of the given params, or an error if any integer param is not an integer. The intParams may be nil if no integer parameters are required. Note this does not check existence; if an integer paramter is required, it should be included in the requiredParams given to NewInfo.

traffic_ops/traffic_ops_golang/crconfig/handler.go

@@ -134,7 +134,7 @@ func SnapshotGetMonitoringHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write([]byte(`{"response":` + snapshot + `}`))
+	api.WriteAndLogErr(w, r, []byte(`{"response":`+snapshot+`}`))
 }
 
 // SnapshotOldGetHandler gets and serves the CRConfig from the snapshot table, not wrapped in response to match the old non-API CRConfig-Snapshots endpoint
@@ -156,7 +156,7 @@ func SnapshotOldGetHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write([]byte(snapshot))
+	api.WriteAndLogErr(w, r, []byte(snapshot))
 }
 
 // SnapshotHandler creates the CRConfig JSON and writes it to the snapshot table in the database.

traffic_ops/traffic_ops_golang/dbdump/dbdump.go

@@ -19,17 +19,18 @@ package dbdump
  * under the License.
  */
 
-import "errors"
-import "fmt"
-import "net/http"
-import "os"
-import "os/exec"
-import "time"
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"os"
+	"os/exec"
+	"time"
 
-import "github.com/apache/trafficcontrol/lib/go-log"
-import "github.com/apache/trafficcontrol/lib/go-rfc"
-
-import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+	"github.com/apache/trafficcontrol/lib/go-log"
+	"github.com/apache/trafficcontrol/lib/go-rfc"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+)
 
 func filename() string {
 	host, err := os.Hostname()
@@ -96,5 +97,5 @@ func DBDump(w http.ResponseWriter, r *http.Request) {
 	if out[len(out)-1] != '\n' {
 		out = append(out, '\n')
 	}
-	w.Write(out)
+	api.WriteAndLogErr(w, r, out)
 }

traffic_ops/traffic_ops_golang/federation_resolvers/federation_resolvers.go

@@ -340,5 +340,5 @@ func DeleteByID(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set(rfc.ContentType, rfc.MIME_JSON.String())
 	w.WriteHeader(statusCode)
-	w.Write(append(respBts, '\n'))
+	api.WriteAndLogErr(w, r, append(respBts, '\n'))
 }

traffic_ops/traffic_ops_golang/federations/federations.go

@@ -580,7 +580,7 @@ func ReplaceFederationResolverMappingsForCurrentUser(w http.ResponseWriter, r *h
 	}
 
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append(respBts, '\n'))
+	api.WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 // retrieves mappings from the given request body using the rules of the given api Version

traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go

@@ -98,7 +98,7 @@ func Get(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append(respBts, '\n'))
+	api.WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 func getHWInfo(tx *sqlx.Tx, params map[string]string) ([]tc.HWInfo, error) {

traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go

@@ -314,7 +314,7 @@ func Create(w http.ResponseWriter, r *http.Request) {
 		}
 
 		w.WriteHeader(http.StatusBadRequest)
-		w.Write(append(resp, '\n'))
+		api.WriteAndLogErr(w, r, append(resp, '\n'))
 		return
 	}
 
@@ -401,7 +401,7 @@ func Create(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set(http.CanonicalHeaderKey("location"), inf.Config.URL.Scheme+"://"+r.Host+"/api/1.4/jobs?id="+strconv.FormatUint(uint64(*result.ID), 10))
 	w.WriteHeader(http.StatusOK)
-	w.Write(append(resp, '\n'))
+	api.WriteAndLogErr(w, r, append(resp, '\n'))
 
 	duplicate := ""
 	if len(conflicts) > 0 {
@@ -574,7 +574,7 @@ func Update(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set(http.CanonicalHeaderKey("content-type"), rfc.ApplicationJSON)
-	w.Write(append(resp, '\n'))
+	api.WriteAndLogErr(w, r, append(resp, '\n'))
 
 	api.CreateChangeLogRawTx(api.ApiChange, api.Updated+" content invalidation job - ID: "+strconv.FormatUint(*job.ID, 10)+" DS: "+*job.DeliveryService+" URL: '"+*job.AssetURL+"' Params: '"+*job.Parameters+"'", inf.User, inf.Tx.Tx)
 }
@@ -660,7 +660,7 @@ func Delete(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set(http.CanonicalHeaderKey("content-type"), rfc.ApplicationJSON)
-	w.Write(append(resp, '\n'))
+	api.WriteAndLogErr(w, r, append(resp, '\n'))
 
 	api.CreateChangeLogRawTx(api.ApiChange, api.Deleted+" content invalidation job - ID: "+strconv.FormatUint(*result.ID, 10)+" DS: "+*result.DeliveryService+" URL: '"+*result.AssetURL+"' Params: '"+*result.Parameters+"'", inf.User, inf.Tx.Tx)
 }

traffic_ops/traffic_ops_golang/login/login.go

@@ -208,7 +208,7 @@ func TokenLoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc {
 		}
 
 		w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-		w.Write(append(respBts, '\n'))
+		api.WriteAndLogErr(w, r, append(respBts, '\n'))
 
 		// TODO: afaik, Perl never clears these tokens. They should be reset to NULL on login, I think.
 	}
@@ -514,6 +514,6 @@ func ResetPassword(db *sqlx.DB, cfg config.Config) http.HandlerFunc {
 		}
 
 		w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-		w.Write(append(respBts, '\n'))
+		api.WriteAndLogErr(w, r, append(respBts, '\n'))
 	}
 }

traffic_ops/traffic_ops_golang/login/logout.go

@@ -19,14 +19,16 @@ package login
  * under the License.
  */
 
-import "encoding/json"
-import "fmt"
-import "net/http"
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
 
-import "github.com/apache/trafficcontrol/lib/go-tc"
-import "github.com/apache/trafficcontrol/lib/go-rfc"
-import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
-import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tocookie"
+	"github.com/apache/trafficcontrol/lib/go-rfc"
+	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tocookie"
+)
 
 func LogoutHandler(secret string) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -53,6 +55,6 @@ func LogoutHandler(secret string) http.HandlerFunc {
 		}
 
 		w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-		w.Write(append(respBts, '\n'))
+		api.WriteAndLogErr(w, r, append(respBts, '\n'))
 	}
 }

traffic_ops/traffic_ops_golang/ping/ping.go

@@ -23,6 +23,7 @@ import (
 	"net/http"
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
 )
 
 const pingResponse = `{"ping":"pong"}`
@@ -31,5 +32,5 @@ const pingResponse = `{"ping":"pong"}`
 // server is responding.
 func Handler(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append([]byte(pingResponse), '\n'))
+	api.WriteAndLogErr(w, r, append([]byte(pingResponse), '\n'))
 }

traffic_ops/traffic_ops_golang/routing/middleware/wrappers.go

@@ -220,7 +220,7 @@ func GzipResponse(w http.ResponseWriter, r *http.Request, bytes []byte) {
 		w.WriteHeader(status)
 	}
 
-	w.Write(bytes)
+	api.WriteAndLogErr(w, r, bytes)
 }
 
 // GzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written.
@@ -252,7 +252,7 @@ func NotImplementedHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 		w.WriteHeader(http.StatusNotImplemented)
-		w.Write([]byte(`{"alerts":[{"level":"error","text":"The requested api version is not implemented by this server. If you are using a newer client with an older server, you will need to use an older client version or upgrade your server."}]}`))
+		api.WriteAndLogErr(w, r, []byte(`{"alerts":[{"level":"error","text":"The requested api version is not implemented by this server. If you are using a newer client with an older server, you will need to use an older client version or upgrade your server."}]}`))
 	})
 }
 
@@ -262,6 +262,6 @@ func DisabledRouteHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 		w.WriteHeader(http.StatusServiceUnavailable)
-		w.Write([]byte(`{"alerts":[{"level":"error","text":"The requested route is currently disabled."}]}` + "\n"))
+		api.WriteAndLogErr(w, r, []byte(`{"alerts":[{"level":"error","text":"The requested route is currently disabled."}]}`+"\n"))
 	})
 }

traffic_ops/traffic_ops_golang/routing/routes.go

@@ -1805,7 +1805,7 @@ func MemoryStatsHandler() http.HandlerFunc {
 			return
 		}
 		w.Header().Set("Content-Type", "application/json")
-		w.Write(bytes)
+		api.WriteAndLogErr(w, r, bytes)
 	}
 }
 
@@ -1821,7 +1821,7 @@ func DBStatsHandler(db *sqlx.DB) http.HandlerFunc {
 			return
 		}
 		w.Header().Set("Content-Type", "application/json")
-		w.Write(bytes)
+		api.WriteAndLogErr(w, r, bytes)
 	}
 }
 
@@ -1843,7 +1843,7 @@ func rootHandler(d ServerData) http.Handler {
 func notImplementedHandler(w http.ResponseWriter, r *http.Request) {
 	code := http.StatusNotImplemented
 	w.WriteHeader(code)
-	w.Write([]byte(http.StatusText(code)))
+	api.WriteAndLogErr(w, r, []byte(http.StatusText(code)))
 }
 
 //CreateThrottledHandler takes a handler, and a max and uses a channel to insure the handler is used concurrently by only max number of routines

traffic_ops/traffic_ops_golang/trafficstats/cache.go

@@ -184,7 +184,7 @@ func GetCacheStats(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set(rfc.ContentType, jsonWithRFCTimestamps.String())
 	}
 	w.Header().Set(http.CanonicalHeaderKey("vary"), http.CanonicalHeaderKey("Accept"))
-	w.Write(append(respBts, '\n'))
+	api.WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 func getCacheSummary(client *influx.Client, conf *tc.TrafficCacheStatsConfig, db string) (*tc.TrafficStatsSummary, error) {

traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go

@@ -247,7 +247,7 @@ func handleRequest(w http.ResponseWriter, r *http.Request, client *influx.Client
 		w.Header().Set(rfc.ContentType, jsonWithRFCTimestamps.String())
 	}
 	w.Header().Set(http.CanonicalHeaderKey("vary"), http.CanonicalHeaderKey("Accept"))
-	w.Write(append(respBts, '\n'))
+	api.WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 func handleLegacyRequest(w http.ResponseWriter, r *http.Request, client *influx.Client, cfg tc.TrafficDSStatsConfig, inf *api.APIInfo) {
@@ -315,7 +315,7 @@ func handleLegacyRequest(w http.ResponseWriter, r *http.Request, client *influx.
 		w.Header().Set(rfc.ContentType, jsonWithRFCTimestamps.String())
 	}
 	w.Header().Set(http.CanonicalHeaderKey("vary"), http.CanonicalHeaderKey("Accept"))
-	w.Write(append(respBts, '\n'))
+	api.WriteAndLogErr(w, r, append(respBts, '\n'))
 }
 
 func getDSSummary(client *influx.Client, conf *tc.TrafficDSStatsConfig, db string) (*tc.TrafficStatsSummary, *float64, *float64, error) {

traffic_ops/traffic_ops_golang/urisigning/urisigning.go

@@ -68,7 +68,7 @@ func GetURIsignkeysHandler(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(ro)
+	api.WriteAndLogErr(w, r, ro)
 }
 
 // removeDeliveryServiceURIKeysHandler is the HTTP DELETE handler used to remove urisigning keys assigned to a delivery service.
@@ -167,7 +167,7 @@ func SaveDeliveryServiceURIKeysHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+xmlID+", ID: "+strconv.Itoa(dsID)+", ACTION: Stored URI signing keys to a delivery service", inf.User, inf.Tx.Tx)
 	w.Header().Set("Content-Type", rfc.ApplicationJSON)
-	w.Write(data)
+	api.WriteAndLogErr(w, r, data)
 }
 
 // getDSIDFromName loads the DeliveryService's ID from the database, from the xml_id. Returns whether the delivery service was found, and any error.