diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b9a2e9cd3..33ba8ff3ab 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go index 3550f8583d..cfc322490e 100644 --- a/traffic_ops/traffic_ops_golang/api/api.go +++ b/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. diff --git a/traffic_ops/traffic_ops_golang/crconfig/handler.go b/traffic_ops/traffic_ops_golang/crconfig/handler.go index 3ba949b751..a550d9827c 100644 --- a/traffic_ops/traffic_ops_golang/crconfig/handler.go +++ b/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. diff --git a/traffic_ops/traffic_ops_golang/dbdump/dbdump.go b/traffic_ops/traffic_ops_golang/dbdump/dbdump.go index 1f00a31b8a..8dc578ab99 100644 --- a/traffic_ops/traffic_ops_golang/dbdump/dbdump.go +++ b/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) } diff --git a/traffic_ops/traffic_ops_golang/federation_resolvers/federation_resolvers.go b/traffic_ops/traffic_ops_golang/federation_resolvers/federation_resolvers.go index b762593b73..9260140087 100644 --- a/traffic_ops/traffic_ops_golang/federation_resolvers/federation_resolvers.go +++ b/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')) } diff --git a/traffic_ops/traffic_ops_golang/federations/federations.go b/traffic_ops/traffic_ops_golang/federations/federations.go index 802bc4eb9c..20ad3949b8 100644 --- a/traffic_ops/traffic_ops_golang/federations/federations.go +++ b/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 diff --git a/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go b/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go index 4957b3f530..fd79880d7c 100644 --- a/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go +++ b/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) { diff --git a/traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go b/traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go index 1f1ef08108..13d3789c76 100644 --- a/traffic_ops/traffic_ops_golang/invalidationjobs/invalidationjobs.go +++ b/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) } diff --git a/traffic_ops/traffic_ops_golang/login/login.go b/traffic_ops/traffic_ops_golang/login/login.go index 0733ba1c29..2f32ed591a 100644 --- a/traffic_ops/traffic_ops_golang/login/login.go +++ b/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')) } } diff --git a/traffic_ops/traffic_ops_golang/login/logout.go b/traffic_ops/traffic_ops_golang/login/logout.go index ba5397719f..955d153b04 100644 --- a/traffic_ops/traffic_ops_golang/login/logout.go +++ b/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')) } } diff --git a/traffic_ops/traffic_ops_golang/ping/ping.go b/traffic_ops/traffic_ops_golang/ping/ping.go index 3f861036dd..8e876e619f 100644 --- a/traffic_ops/traffic_ops_golang/ping/ping.go +++ b/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')) } diff --git a/traffic_ops/traffic_ops_golang/routing/middleware/wrappers.go b/traffic_ops/traffic_ops_golang/routing/middleware/wrappers.go index 7ac44e9ae7..b70f20a42f 100644 --- a/traffic_ops/traffic_ops_golang/routing/middleware/wrappers.go +++ b/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")) }) } diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 32bb244e0c..7b5326b828 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/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 diff --git a/traffic_ops/traffic_ops_golang/trafficstats/cache.go b/traffic_ops/traffic_ops_golang/trafficstats/cache.go index 1a86ade3fe..c03b8042fd 100644 --- a/traffic_ops/traffic_ops_golang/trafficstats/cache.go +++ b/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) { diff --git a/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go b/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go index dece1a3973..cf66b2eb9a 100644 --- a/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go +++ b/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) { diff --git a/traffic_ops/traffic_ops_golang/urisigning/urisigning.go b/traffic_ops/traffic_ops_golang/urisigning/urisigning.go index 443974b9c9..9984dba4cf 100644 --- a/traffic_ops/traffic_ops_golang/urisigning/urisigning.go +++ b/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.