[#328] Log error on failed response writing

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>

api/handler/cors.go

@@ -66,7 +66,10 @@ func (h *handler) PutBucketCorsHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	middleware.WriteSuccessResponseHeadersOnly(w)
+	if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
+		h.logAndSendError(w, "write response", reqInfo, err)
+		return
+	}
 }
 
 func (h *handler) DeleteBucketCorsHandler(w http.ResponseWriter, r *http.Request) {
@@ -200,7 +203,10 @@ func (h *handler) Preflight(w http.ResponseWriter, r *http.Request) {
 						if o != wildcard {
 							w.Header().Set(api.AccessControlAllowCredentials, "true")
 						}
-						middleware.WriteSuccessResponseHeadersOnly(w)
+						if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
+							h.logAndSendError(w, "write response", reqInfo, err)
+							return
+						}
 						return
 					}
 				}

api/handler/head.go

@@ -140,7 +140,10 @@ func (h *handler) HeadBucketHandler(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set(api.ContainerZone, bktInfo.Zone)
 	}
 
-	middleware.WriteResponse(w, http.StatusOK, nil, middleware.MimeNone)
+	if err = middleware.WriteResponse(w, http.StatusOK, nil, middleware.MimeNone); err != nil {
+		h.logAndSendError(w, "write response", reqInfo, err)
+		return
+	}
 }
 
 func (h *handler) setLockingHeaders(bktInfo *data.BucketInfo, lockInfo data.LockInfo, header http.Header) error {

api/handler/multipart_upload.go

@@ -266,7 +266,10 @@ func (h *handler) UploadPartHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set(api.ETag, data.Quote(hash))
-	middleware.WriteSuccessResponseHeadersOnly(w)
+	if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
+		h.logAndSendError(w, "write response", reqInfo, err)
+		return
+	}
 }
 
 func (h *handler) UploadPartCopy(w http.ResponseWriter, r *http.Request) {

api/handler/put.go

@@ -337,7 +337,10 @@ func (h *handler) PutObjectHandler(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set(api.ETag, data.Quote(objInfo.ETag(h.cfg.MD5Enabled())))
 
-	middleware.WriteSuccessResponseHeadersOnly(w)
+	if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
+		h.logAndSendError(w, "write response", reqInfo, err)
+		return
+	}
 }
 
 func (h *handler) getBodyReader(r *http.Request) (io.ReadCloser, error) {
@@ -602,7 +605,11 @@ func (h *handler) PostObject(w http.ResponseWriter, r *http.Request) {
 				ETag:   data.Quote(objInfo.ETag(h.cfg.MD5Enabled())),
 			}
 			w.WriteHeader(status)
-			if _, err = w.Write(middleware.EncodeResponse(resp)); err != nil {
+			respData, err := middleware.EncodeResponse(resp)
+			if err != nil {
+				h.logAndSendError(w, "encode response", reqInfo, err)
+			}
+			if _, err = w.Write(respData); err != nil {
 				h.logAndSendError(w, "something went wrong", reqInfo, err)
 			}
 			return
@@ -873,7 +880,10 @@ func (h *handler) createBucketHandlerPolicy(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	middleware.WriteSuccessResponseHeadersOnly(w)
+	if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
+		h.logAndSendError(w, "write response", reqInfo, err)
+		return
+	}
 }
 
 func (h *handler) createBucketHandlerACL(w http.ResponseWriter, r *http.Request) {
@@ -942,7 +952,10 @@ func (h *handler) createBucketHandlerACL(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	middleware.WriteSuccessResponseHeadersOnly(w)
+	if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
+		h.logAndSendError(w, "write response", reqInfo, err)
+		return
+	}
 }
 
 const s3ActionPrefix = "s3:"

api/handler/util.go

@@ -31,9 +31,12 @@ func (h *handler) reqLogger(ctx context.Context) *zap.Logger {
 
 func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo *middleware.ReqInfo, err error, additional ...zap.Field) {
 	err = handleDeleteMarker(w, err)
-	code := middleware.WriteErrorResponse(w, reqInfo, transformToS3Error(err))
+	if code, wrErr := middleware.WriteErrorResponse(w, reqInfo, transformToS3Error(err)); wrErr != nil {
+		additional = append(additional, zap.NamedError("write_response_error", wrErr))
+	} else {
+		additional = append(additional, zap.Int("status", code))
+	}
 	fields := []zap.Field{
-		zap.Int("status", code),
 		zap.String("request_id", reqInfo.RequestID),
 		zap.String("method", reqInfo.API),
 		zap.String("bucket", reqInfo.BucketName),

api/middleware/auth.go

@@ -59,7 +59,9 @@ func Auth(center Center, log *zap.Logger) Func {
 					if _, ok := err.(apiErrors.Error); !ok {
 						err = apiErrors.GetAPIError(apiErrors.ErrAccessDenied)
 					}
-					WriteErrorResponse(w, GetReqInfo(r.Context()), err)
+					if _, wrErr := WriteErrorResponse(w, GetReqInfo(r.Context()), err); wrErr != nil {
+						reqLogOrDefault(ctx, log).Error(logs.FailedToWriteResponse, zap.Error(wrErr))
+					}
 					return
 				}
 			} else {
@@ -97,7 +99,9 @@ func FrostfsIDValidation(frostfsID FrostFSIDValidator, log *zap.Logger) Func {
 
 			if err = validateBearerToken(frostfsID, bd.Gate.BearerToken); err != nil {
 				reqLogOrDefault(ctx, log).Error(logs.FrostfsIDValidationFailed, zap.Error(err))
-				WriteErrorResponse(w, GetReqInfo(r.Context()), err)
+				if _, wrErr := WriteErrorResponse(w, GetReqInfo(r.Context()), err); wrErr != nil {
+					reqLogOrDefault(ctx, log).Error(logs.FailedToWriteResponse, zap.Error(wrErr))
+				}
 				return
 			}
 

api/middleware/policy.go

@@ -47,7 +47,9 @@ func PolicyCheck(cfg PolicyConfig) Func {
 			ctx := r.Context()
 			if err := policyCheck(r, cfg); err != nil {
 				reqLogOrDefault(ctx, cfg.Log).Error(logs.PolicyValidationFailed, zap.Error(err))
-				WriteErrorResponse(w, GetReqInfo(ctx), err)
+				if _, wrErr := WriteErrorResponse(w, GetReqInfo(ctx), err); wrErr != nil {
+					reqLogOrDefault(ctx, cfg.Log).Error(logs.FailedToWriteResponse, zap.Error(wrErr))
+				}
 				return
 			}
 

api/middleware/response.go

@@ -118,7 +118,8 @@ var s3ErrorResponseMap = map[string]string{
 }
 
 // WriteErrorResponse writes error headers.
-func WriteErrorResponse(w http.ResponseWriter, reqInfo *ReqInfo, err error) int {
+// returns http error code and error in case of failure of response writing.
+func WriteErrorResponse(w http.ResponseWriter, reqInfo *ReqInfo, err error) (int, error) {
 	code := http.StatusInternalServerError
 
 	if e, ok := err.(errors.Error); ok {
@@ -134,9 +135,14 @@ func WriteErrorResponse(w http.ResponseWriter, reqInfo *ReqInfo, err error) int
 
 	// Generates error response.
 	errorResponse := getAPIErrorResponse(reqInfo, err)
-	encodedErrorResponse := EncodeResponse(errorResponse)
-	WriteResponse(w, code, encodedErrorResponse, MimeXML)
-	return code
+	encodedErrorResponse, err := EncodeResponse(errorResponse)
+	if err != nil {
+		return 0, fmt.Errorf("encode response: %w", err)
+	}
+	if err = WriteResponse(w, code, encodedErrorResponse, MimeXML); err != nil {
+		return 0, fmt.Errorf("write response: %w", err)
+	}
+	return code, nil
 }
 
 // Write http common headers.
@@ -157,7 +163,7 @@ func removeSensitiveHeaders(h http.Header) {
 }
 
 // WriteResponse writes given statusCode and response into w (with mType header if set).
-func WriteResponse(w http.ResponseWriter, statusCode int, response []byte, mType mimeType) {
+func WriteResponse(w http.ResponseWriter, statusCode int, response []byte, mType mimeType) error {
 	setCommonHeaders(w)
 	if mType != MimeNone {
 		w.Header().Set(hdrContentType, string(mType))
@@ -165,37 +171,46 @@ func WriteResponse(w http.ResponseWriter, statusCode int, response []byte, mType
 	w.Header().Set(hdrContentLength, strconv.Itoa(len(response)))
 	w.WriteHeader(statusCode)
 	if response == nil {
-		return
+		return nil
 	}
 
-	WriteResponseBody(w, response)
+	return WriteResponseBody(w, response)
 }
 
 // WriteResponseBody writes response into w.
-func WriteResponseBody(w http.ResponseWriter, response []byte) {
-	_, _ = w.Write(response)
+func WriteResponseBody(w http.ResponseWriter, response []byte) error {
+	if _, err := w.Write(response); err != nil {
+		return err
+	}
+
 	if flusher, ok := w.(http.Flusher); ok {
 		flusher.Flush()
 	}
+
+	return nil
 }
 
 // EncodeResponse encodes the response headers into XML format.
-func EncodeResponse(response interface{}) []byte {
+func EncodeResponse(response interface{}) ([]byte, error) {
 	var bytesBuffer bytes.Buffer
 	bytesBuffer.WriteString(xml.Header)
-	_ = xml.
-		NewEncoder(&bytesBuffer).
-		Encode(response)
-	return bytesBuffer.Bytes()
+	if err := xml.NewEncoder(&bytesBuffer).Encode(response); err != nil {
+		return nil, err
+	}
+
+	return bytesBuffer.Bytes(), nil
 }
 
 // EncodeResponseNoHeader encodes response without setting xml.Header.
 // Should be used with periodicXMLWriter which sends xml.Header to the client
 // with whitespaces to keep connection alive.
-func EncodeResponseNoHeader(response interface{}) []byte {
+func EncodeResponseNoHeader(response interface{}) ([]byte, error) {
 	var bytesBuffer bytes.Buffer
-	_ = xml.NewEncoder(&bytesBuffer).Encode(response)
-	return bytesBuffer.Bytes()
+	if err := xml.NewEncoder(&bytesBuffer).Encode(response); err != nil {
+		return nil, err
+	}
+
+	return bytesBuffer.Bytes(), nil
 }
 
 // EncodeToResponse encodes the response into ResponseWriter.
@@ -227,8 +242,8 @@ func EncodeToResponseNoHeader(w http.ResponseWriter, response interface{}) error
 
 // WriteSuccessResponseHeadersOnly writes HTTP (200) OK response with no data
 // to the client.
-func WriteSuccessResponseHeadersOnly(w http.ResponseWriter) {
-	WriteResponse(w, http.StatusOK, nil, MimeNone)
+func WriteSuccessResponseHeadersOnly(w http.ResponseWriter) error {
+	return WriteResponse(w, http.StatusOK, nil, MimeNone)
 }
 
 // Error -- Returns S3 error string.

api/router.go

@@ -178,14 +178,24 @@ func errorResponseHandler(w http.ResponseWriter, r *http.Request) {
 	reqInfo := s3middleware.GetReqInfo(ctx)
 
 	desc := fmt.Sprintf("Unknown API request at %s", r.URL.Path)
-	s3middleware.WriteErrorResponse(w, reqInfo, errors.Error{
+	_, wrErr := s3middleware.WriteErrorResponse(w, reqInfo, errors.Error{
 		Code:           "UnknownAPIRequest",
 		Description:    desc,
 		HTTPStatusCode: http.StatusBadRequest,
 	})
 
 	if log := s3middleware.GetReqLog(ctx); log != nil {
-		log.Error(logs.RequestUnmatched, zap.String("method", reqInfo.API), zap.String("http method", r.Method), zap.String("url", r.RequestURI))
+		fields := []zap.Field{
+			zap.String("method", reqInfo.API),
+			zap.String("http method", r.Method),
+			zap.String("url", r.RequestURI),
+		}
+
+		if wrErr != nil {
+			fields = append(fields, zap.NamedError("write_response_error", wrErr))
+		}
+
+		log.Error(logs.RequestUnmatched, fields...)
 	}
 }
 

internal/logs/logs.go

@@ -149,4 +149,5 @@ const (
 	FailedToGenerateRequestID                            = "failed to generate request id"
 	InvalidBucketObjectLockEnabledHeader                 = "invalid X-Amz-Bucket-Object-Lock-Enabled header"
 	InvalidTreeKV                                        = "invalid tree service meta KV"
+	FailedToWriteResponse                                = "failed to write response"
 )