Files
seaweedfs/weed/s3api/iceberg/log_manager_error_test.go
T
Javier de la TorreandJavier de la Torre 97f8ca13ff s3tables, iceberg: log client refusals at V(1), not ERROR (#11896)
* s3tables, iceberg: log client refusals at V(1), not ERROR

Requests the S3 Tables and Iceberg REST APIs correctly refuse with a 4xx
(access denied, not authorized, no such namespace, already exists, not
empty, a commit conflict) were logged with glog.Errorf, so a client probing
for what it may do, or retrying an optimistic commit, filled the ERROR log
and hid real faults from anyone alerting on it.

- s3tables: HandleRequest wraps the ResponseWriter in a statusRecorder (with
  Unwrap for http.ResponseController) and logs a handler error by the status
  it answered: 4xx at V(1) as "<op> refused (<status>)", 5xx or no status at
  ERROR as before.
- s3tables: the "handleCreateNamespace called" trace moves from ERROR to V(3).
- iceberg: writeManagerError's mapping is split into classifyManagerError,
  shared with a new logManagerError that logs 4xx-mapped errors at V(1) and
  everything else at ERROR; used by CreateNamespace, CommitTable and
  CommitTransaction.
- storage: loading a remote-tier volume logged
  "readSuperBlock remote volume N: <nil>" at ERROR even when the read
  succeeded; it is now logged only when there is an error.

Tests: TestRequestRefusedWith4xxIsNotLoggedAsError, TestLogRequestErrorByStatus,
TestStatusRecorderKeepsTheFirstStatus, TestStatusRecorderUnwrapsForResponseController,
TestNoTraceLoggedAtErrorLevel (fails on master on the CreateNamespace trace),
TestLogManagerErrorRefusalsAreNotErrors, TestRemoteSuperBlockLogOnlyOnError.

* s3tables: do not log a malformed CreateNamespace body at ERROR

handleCreateNamespace logged a body it could not decode at ERROR before
answering 400; the dispatcher already logs that refusal once at V(1).

Also drop statusRecorder.Flush: it made the recorder an http.Flusher even
over a writer that cannot flush, so http.NewResponseController reported
success instead of ErrNotSupported. Unwrap already lets the controller
reach the wrapped writer.

* iceberg: require exactly one log line per classified manager error

The test accepted a refusal that produced no log, or both logs. Require
one V(1) refusal and no ERROR for a 4xx, and the reverse for a fault.

---------

Co-authored-by: Javier de la Torre <jatorre@lakehousebox.com>
2026-10-21 14:00:33 -07:00

37 lines
1.4 KiB
Go

package iceberg
import (
"errors"
"fmt"
"testing"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3tables"
)
func TestLogManagerErrorRefusalsAreNotErrors(t *testing.T) {
var refusals, faults []string
oldR, oldF := logManagerRefusal, logManagerFault
logManagerRefusal = func(f string, a ...interface{}) { refusals = append(refusals, fmt.Sprintf(f, a...)) }
logManagerFault = func(f string, a ...interface{}) { faults = append(faults, fmt.Sprintf(f, a...)) }
defer func() { logManagerRefusal, logManagerFault = oldR, oldF }()
for _, c := range []struct {
err error
fault bool
}{
{&s3tables.S3TablesError{Type: s3tables.ErrCodeAccessDenied, Message: "not authorized to create namespace in this bucket"}, false},
{&s3tables.S3TablesError{Type: s3tables.ErrCodeNoSuchNamespace, Message: "no such namespace"}, false},
{&s3tables.S3TablesError{Type: s3tables.ErrCodeNamespaceAlreadyExists, Message: "exists"}, false},
{errors.New("filer: connection refused"), true}, // positive control: a real fault stays at ERROR
} {
refusals, faults = nil, nil
logManagerError("CreateNamespace", c.err)
wantFaults, wantRefusals := 0, 1
if c.fault {
wantFaults, wantRefusals = 1, 0
}
if len(faults) != wantFaults || len(refusals) != wantRefusals {
t.Errorf("%v: %d ERROR and %d refusal log(s), want %d and %d (refusals %v, faults %v)", c.err, len(faults), len(refusals), wantFaults, wantRefusals, refusals, faults)
}
}
}