When writing to logging destination fails, fleet server now issues a 4xx error instead of 500. (#16420)

changes/5017-convert-logging-destination-err-to-4xx

@@ -0,0 +1 @@
+When writing to logging destination fails, fleet server now issues a 4xx error instead of 500.

server/service/osquery.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/http"
 	"regexp"
 	"strconv"
 	"strings"
@@ -29,7 +30,7 @@ import (
 type osqueryError struct {
 	message     string
 	nodeInvalid bool
-
+	statusCode  int
 	fleet.ErrorWithUUID
 }
 
@@ -46,6 +47,10 @@ func (e *osqueryError) NodeInvalid() bool {
 	return e.nodeInvalid
 }
 
+func (e *osqueryError) Status() int {
+	return e.statusCode
+}
+
 func newOsqueryErrorWithInvalidNode(msg string) *osqueryError {
 	return &osqueryError{
 		message:     msg,
@@ -1539,7 +1544,10 @@ func (svc *Service) SubmitStatusLogs(ctx context.Context, logs []json.RawMessage
 	svc.authz.SkipAuthorization(ctx)
 
 	if err := svc.osqueryLogWriter.Status.Write(ctx, logs); err != nil {
-		return newOsqueryError("error writing status logs: " + err.Error())
+		osqueryErr := newOsqueryError("error writing status logs: " + err.Error())
+		// Attempting to write a large amount of data is the most likely explanation for this error.
+		osqueryErr.statusCode = http.StatusRequestEntityTooLarge
+		return osqueryErr
 	}
 	return nil
 }
@@ -1616,11 +1624,14 @@ func (svc *Service) SubmitResultLogs(ctx context.Context, logs []json.RawMessage
 	}
 
 	if err := svc.osqueryLogWriter.Result.Write(ctx, filteredLogs); err != nil {
-		return newOsqueryError(
+		osqueryErr := newOsqueryError(
 			"error writing result logs " +
 				"(if the logging destination is down, you can reduce frequency/size of osquery logs by " +
 				"increasing logger_tls_period and decreasing logger_tls_max_lines): " + err.Error(),
 		)
+		// Attempting to write a large amount of data is the most likely explanation for this error.
+		osqueryErr.statusCode = http.StatusRequestEntityTooLarge
+		return osqueryErr
 	}
 	return nil
 }

server/service/osquery_test.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"reflect"
 	"sort"
@@ -840,6 +841,62 @@ func TestSubmitResultLogsToQueryResultsDoesNotCountNullDataRows(t *testing.T) {
 	assert.True(t, ds.OverwriteQueryResultRowsFuncInvoked)
 }
 
+type failingLogger struct {
+}
+
+func (n *failingLogger) Write(context.Context, []json.RawMessage) error {
+	return errors.New("some error")
+}
+
+func TestSubmitResultLogsFail(t *testing.T) {
+	ds := new(mock.Store)
+	svc, ctx := newTestService(t, ds, nil, nil)
+
+	host := fleet.Host{
+		ID: 999,
+	}
+	ctx = hostctx.NewContext(ctx, &host)
+
+	// Hack to get at the service internals and modify the writer
+	serv := ((svc.(validationMiddleware)).Service).(*Service)
+
+	testLogger := &failingLogger{}
+	serv.osqueryLogWriter = &OsqueryLogger{Result: testLogger}
+
+	logs := []string{
+		`{"name":"pack/Global/system_info","hostIdentifier":"some_uuid","calendarTime":"Fri Sep 30 17:55:15 2016 UTC","unixTime":1475258115,"decorations":{"host_uuid":"some_uuid","username":"zwass"},"columns":{"cpu_brand":"Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz","hostname":"hostimus","physical_memory":"17179869184"},"action":"added"}`,
+	}
+
+	logJSON := fmt.Sprintf("[%s]", strings.Join(logs, ","))
+	var results []json.RawMessage
+	err := json.Unmarshal([]byte(logJSON), &results)
+	require.NoError(t, err)
+
+	ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
+		return &fleet.AppConfig{}, nil
+	}
+	ds.QueryByNameFunc = func(ctx context.Context, teamID *uint, name string) (*fleet.Query, error) {
+		return &fleet.Query{
+			ID:                 1,
+			DiscardData:        false,
+			AutomationsEnabled: true,
+			Name:               name,
+		}, nil
+	}
+	ds.ResultCountForQueryFunc = func(ctx context.Context, queryID uint) (int, error) {
+		return 0, nil
+	}
+	ds.OverwriteQueryResultRowsFunc = func(ctx context.Context, rows []*fleet.ScheduledQueryResultRow) error {
+		return nil
+	}
+
+	// Expect an error when unable to write to logging destination.
+	err = svc.SubmitResultLogs(ctx, results)
+	require.Error(t, err)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, err.(*osqueryError).Status())
+
+}
+
 func TestGetQueryNameAndTeamIDFromResult(t *testing.T) {
 	tests := []struct {
 		input        string

server/service/transport_error.go

@@ -132,6 +132,8 @@ func encodeError(ctx context.Context, err error, w http.ResponseWriter) {
 		if e.NodeInvalid() {
 			w.WriteHeader(http.StatusUnauthorized)
 			errMap["node_invalid"] = true
+		} else if e.Status() != 0 {
+			w.WriteHeader(e.Status())
 		} else {
 			// TODO: osqueryError is not always the result of an internal error on
 			// our side, it is also used to represent a client error (invalid data,