From f78160a69d965c95186a5ae587e94037b477a83b Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Thu, 3 Apr 2025 08:44:01 +0400 Subject: [PATCH] Remove error middleware for now (#5008) Closes #4965. --- .golangci-new.yml | 2 +- .golangci.yml | 7 ++ build/deps/postgres-documentdb.Dockerfile | 4 +- build/postgres-documentdb/documentdb | 2 +- build/version/documentdb.go | 2 +- integration/op_query_test.go | 4 +- internal/clientconn/conn.go | 64 ++++++++-------- internal/handler/cmd_query.go | 12 +-- internal/handler/middleware/error.go | 91 ----------------------- internal/handler/middleware/middleware.go | 15 +--- website/docs/telemetry.md | 2 +- 11 files changed, 52 insertions(+), 153 deletions(-) delete mode 100644 internal/handler/middleware/error.go diff --git a/.golangci-new.yml b/.golangci-new.yml index 1652e038..25b58b77 100644 --- a/.golangci-new.yml +++ b/.golangci-new.yml @@ -62,6 +62,7 @@ linters: - asciicheck - depguard - exhaustive + - exhaustruct - forbidigo - gci - gochecksumtype @@ -98,7 +99,6 @@ linters: - err113 - errchkjson - errname - - exhaustruct - fatcontext - forcetypeassert - funlen diff --git a/.golangci.yml b/.golangci.yml index 92de0c51..f98130a6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -72,6 +72,12 @@ linters-settings: - pkg: github.com/FerretDB/FerretDB/v2/internal/documentdb/cursor exhaustive: default-signifies-exhaustive: false + exhaustruct: + include: + - .*\Q/middleware.MsgRequest + - .*\Q/middleware.MsgResponse + - .*\Q/middleware.QueryRequest + - .*\Q/middleware.ReplyResponse forbidigo: forbid: - p: ^\Qpgxpool.Pool.AcquireFunc\E$ @@ -239,6 +245,7 @@ linters: - asciicheck - depguard - exhaustive + - exhaustruct - forbidigo - gci - gochecksumtype diff --git a/build/deps/postgres-documentdb.Dockerfile b/build/deps/postgres-documentdb.Dockerfile index a3bcf815..7dea806c 100644 --- a/build/deps/postgres-documentdb.Dockerfile +++ b/build/deps/postgres-documentdb.Dockerfile @@ -1,5 +1,5 @@ # Use production image and full tag close to the release. -# FROM ghcr.io/ferretdb/postgres-documentdb:17-0.102.0-ferretdb-2.0.0 +FROM ghcr.io/ferretdb/postgres-documentdb:17-0.102.0-ferretdb-2.1.0 # Use moving development image during development. -FROM ghcr.io/ferretdb/postgres-documentdb-dev:17-ferretdb +# FROM ghcr.io/ferretdb/postgres-documentdb-dev:17-ferretdb diff --git a/build/postgres-documentdb/documentdb b/build/postgres-documentdb/documentdb index e44d4000..80462f5c 160000 --- a/build/postgres-documentdb/documentdb +++ b/build/postgres-documentdb/documentdb @@ -1 +1 @@ -Subproject commit e44d40009c1397890ff0ede7e76c769195fa120d +Subproject commit 80462f5ce84701d281d5081d11bc54079c0d82c5 diff --git a/build/version/documentdb.go b/build/version/documentdb.go index d1d3ce31..fc7f46a4 100644 --- a/build/version/documentdb.go +++ b/build/version/documentdb.go @@ -20,7 +20,7 @@ const ( "compiled by gcc (Debian 12.2.0-14) 12.2.0, 64-bit" // DocumentDB is a version of DocumentDB this version of FerretDB is compatible with. - DocumentDB = "0.102.0 gitref: ferretdb sha:e44d400 buildId:0" + DocumentDB = "0.102.0 gitref: HEAD sha:80462f5 buildId:0" // DocumentDBURL points to the release page of the DocumentDB version above. DocumentDBURL = "https://github.com/FerretDB/documentdb/releases/tag/v0.102.0-ferretdb-2.1.0" diff --git a/integration/op_query_test.go b/integration/op_query_test.go index 52040dec..f4e3b74a 100644 --- a/integration/op_query_test.go +++ b/integration/op_query_test.go @@ -55,7 +55,7 @@ func TestOpQuery(t *testing.T) { code := resMsg.Get("code") assert.Equal(t, int32(5739101), code) - expectedMsg := "OP_QUERY is no longer supported. The client driver may require an upgrade." + expectedMsg := "OP_QUERY is no longer supported." resErr := resMsg.Get("$err") assert.Contains(t, resErr, expectedMsg) }) @@ -77,7 +77,7 @@ func TestOpQuery(t *testing.T) { code := resMsg.Get("code") assert.Equal(t, int32(352), code) - expectedMsg := "Unsupported OP_QUERY command: unknown. The client driver may require an upgrade." + expectedMsg := "Unsupported OP_QUERY command: unknown." errMsg := resMsg.Get("errmsg") assert.Contains(t, errMsg, expectedMsg) }) diff --git a/internal/clientconn/conn.go b/internal/clientconn/conn.go index 10541cc6..1b634689 100644 --- a/internal/clientconn/conn.go +++ b/internal/clientconn/conn.go @@ -359,6 +359,7 @@ func (c *conn) route(connCtx context.Context, reqHeader *wire.MsgHeader, reqBody }() resHeader = new(wire.MsgHeader) + var err error switch reqHeader.OpCode { case wire.OpCodeMsg: @@ -387,28 +388,39 @@ func (c *conn) route(connCtx context.Context, reqHeader *wire.MsgHeader, reqBody connCtx, span = otel.Tracer("").Start(connCtx, "") if err == nil { - resMsg := c.handleOpMsg(connCtx, &middleware.MsgRequest{OpMsg: msg}, command) - resBody = resMsg.OpMsg + var cmdHandler middleware.MsgHandlerFunc + + cmd, ok := c.h.Commands()[command] + if ok && cmd.Handler != nil { + cmdHandler = cmd.Handler + } else { + cmdHandler = notFound(command) + } + + var res *middleware.MsgResponse + res, err = cmdHandler(connCtx, &middleware.MsgRequest{OpMsg: msg}) + + if res != nil { + resBody = res.OpMsg + } } case wire.OpCodeQuery: - connCtx, span = otel.Tracer("").Start(connCtx, "") - query := reqBody.(*wire.OpQuery) + resHeader.OpCode = wire.OpCodeReply - errMW := middleware.NewError("", logging.WithName(c.l, "error")) + command = query.Query().Command() - replyHandler := c.h.CmdQuery - replyHandler = errMW.HandleOpReply(replyHandler) + connCtx, span = otel.Tracer("").Start(connCtx, "") - // error is handled by middleware.Error - reply := must.NotFail(replyHandler(connCtx, &middleware.QueryRequest{OpQuery: query})) - resBody = reply.OpReply + cmdHandler := c.h.CmdQuery - if replyErr := reply.CommandError(); replyErr != nil { - result = replyErr.Name - argument = replyErr.Argument + var res *middleware.ReplyResponse + res, err = cmdHandler(connCtx, &middleware.QueryRequest{OpQuery: query}) + + if res != nil { + resBody = res.OpReply } case wire.OpCodeReply: @@ -450,6 +462,11 @@ func (c *conn) route(connCtx context.Context, reqHeader *wire.MsgHeader, reqBody argument = protoErr.Argument case wire.OpCodeReply: + protoErr := mongoerrors.Make(connCtx, err, "", c.l) + resBody = protoErr.Reply() + result = protoErr.Name + argument = protoErr.Argument + case wire.OpCodeQuery: fallthrough case wire.OpCodeUpdate: @@ -511,27 +528,6 @@ func (c *conn) route(connCtx context.Context, reqHeader *wire.MsgHeader, reqBody return } -// handleOpMsg processes OP_MSG requests. -// -// The passed context is canceled when the client disconnects. -func (c *conn) handleOpMsg(connCtx context.Context, msg *middleware.MsgRequest, command string) *middleware.MsgResponse { - var cmdHandler middleware.MsgHandlerFunc - - cmd, ok := c.h.Commands()[command] - if !ok || cmd.Handler == nil { - cmdHandler = notFound(command) - } else { - cmdHandler = cmd.Handler - } - - errMW := middleware.NewError("", logging.WithName(c.l, "error")) - - cmdHandler = errMW.HandleOpMsg(cmdHandler) - - // error is handled by middleware.Error - return must.NotFail(cmdHandler(connCtx, msg)) -} - // notFound returns a handler that returns an error indicating that the command is not found. func notFound(command string) middleware.MsgHandlerFunc { return func(context.Context, *middleware.MsgRequest) (*middleware.MsgResponse, error) { diff --git a/internal/handler/cmd_query.go b/internal/handler/cmd_query.go index 634468c9..8e37d444 100644 --- a/internal/handler/cmd_query.go +++ b/internal/handler/cmd_query.go @@ -32,24 +32,24 @@ import ( // // The passed context is canceled when the client connection is closed. func (h *Handler) CmdQuery(connCtx context.Context, query *middleware.QueryRequest) (*middleware.ReplyResponse, error) { - q := query.Query() + q := query.OpQuery.Query() cmd := q.Command() - collection := query.FullCollectionName + collection := query.OpQuery.FullCollectionName suffix := ".$cmd" if !strings.HasSuffix(collection, suffix) { // TODO https://github.com/FerretDB/FerretDB-DocumentDB/issues/527 return middleware.Reply(wirebson.MustDocument( - "$err", "OP_QUERY is no longer supported. The client driver may require an upgrade.", + "$err", "OP_QUERY is no longer supported. The client driver may require an update.", "code", int32(mongoerrors.ErrLocation5739101), "ok", float64(0), )) } - if query.NumberToReturn != 1 && query.NumberToReturn != -1 { + if query.OpQuery.NumberToReturn != 1 && query.OpQuery.NumberToReturn != -1 { return nil, mongoerrors.NewWithArgument( mongoerrors.ErrLocation16979, - fmt.Sprintf("Bad numberToReturn (%d) for $cmd type ns - can only be 1 or -1", query.NumberToReturn), + fmt.Sprintf("Bad numberToReturn (%d) for $cmd type ns - can only be 1 or -1", query.OpQuery.NumberToReturn), "OpQuery: "+cmd, ) } @@ -100,7 +100,7 @@ func (h *Handler) CmdQuery(connCtx context.Context, query *middleware.QueryReque return nil, mongoerrors.NewWithArgument( mongoerrors.ErrUnsupportedOpQueryCommand, - fmt.Sprintf("Unsupported OP_QUERY command: %s. The client driver may require an upgrade.", cmd), + fmt.Sprintf("Unsupported OP_QUERY command: %s. The client driver may require an update.", cmd), "OpQuery: "+cmd, ) } diff --git a/internal/handler/middleware/error.go b/internal/handler/middleware/error.go deleted file mode 100644 index ea1567b2..00000000 --- a/internal/handler/middleware/error.go +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2021 FerretDB Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package middleware - -import ( - "context" - "log/slog" - - "github.com/FerretDB/FerretDB/v2/internal/mongoerrors" -) - -// Error is a middleware that converts an error into a response. -type Error struct { - l *slog.Logger - arg string -} - -// NewError creates a new Error middleware. -func NewError(arg string, l *slog.Logger) *Error { - return &Error{ - arg: arg, - l: l, - } -} - -// HandleOpMsg implements Middleware. -// -// If an error was returned from `next` handler, it converts the error to OP_MSG response. -// It also sets the Error field in the response, for the later use by such as observability. -// Returned MsgHandlerFunc always returns nil error. -func (e *Error) HandleOpMsg(next MsgHandlerFunc) MsgHandlerFunc { - return func(ctx context.Context, req *MsgRequest) (resp *MsgResponse, err error) { - defer func() { - if err != nil { - mErr := mongoerrors.Make(ctx, err, e.arg, e.l) - - resp = &MsgResponse{ - OpMsg: mErr.Msg(), - err: mErr, - } - - err = nil - } - }() - - resp, err = next(ctx, req) - - return - } -} - -// HandleOpReply implements Middleware. -// -// If an error was returned from `next` handler, it converts the error to OP_REPLY response. -// It also sets the Error field in the response, for the later use by such as observability. -// Returned QueryHandlerFunc always returns nil error. -func (e *Error) HandleOpReply(next QueryHandlerFunc) QueryHandlerFunc { - return func(ctx context.Context, req *QueryRequest) (resp *ReplyResponse, err error) { - defer func() { - if err != nil { - mErr := mongoerrors.Make(ctx, err, e.arg, e.l) - - resp = &ReplyResponse{ - OpReply: mErr.Reply(), - err: mErr, - } - - err = nil - } - }() - - resp, err = next(ctx, req) - - return - } -} - -// check interface. -var _ Middleware = (*Error)(nil) diff --git a/internal/handler/middleware/middleware.go b/internal/handler/middleware/middleware.go index 7f172689..4844964c 100644 --- a/internal/handler/middleware/middleware.go +++ b/internal/handler/middleware/middleware.go @@ -21,7 +21,6 @@ import ( "github.com/FerretDB/wire" "github.com/FerretDB/wire/wirebson" - "github.com/FerretDB/FerretDB/v2/internal/mongoerrors" "github.com/FerretDB/FerretDB/v2/internal/util/lazyerrors" ) @@ -34,19 +33,17 @@ type MsgRequest struct { // MsgResponse represent outgoing response to the client. type MsgResponse struct { *wire.OpMsg - err *mongoerrors.Error } // QueryRequest is a deprecated request message type. // It is still used by commands including `hello` and `isMaster`. type QueryRequest struct { - *wire.OpQuery + OpQuery *wire.OpQuery } // ReplyResponse is a deprecated response message type used for the response to [QueryRequest]. type ReplyResponse struct { OpReply *wire.OpReply - err *mongoerrors.Error } // Middleware represents functions for handling incoming requests. @@ -65,11 +62,6 @@ func Response(doc wirebson.AnyDocument) (*MsgResponse, error) { return &MsgResponse{OpMsg: msg}, nil } -// CommandError returns [*mongoerrors.Error] from the response. -func (r *MsgResponse) CommandError() *mongoerrors.Error { - return r.err -} - // Reply constructs a [*ReplyResponse] from a single document. func Reply(doc wirebson.AnyDocument) (*ReplyResponse, error) { reply, err := wire.NewOpReply(doc) @@ -80,11 +72,6 @@ func Reply(doc wirebson.AnyDocument) (*ReplyResponse, error) { return &ReplyResponse{OpReply: reply}, nil } -// CommandError returns [*mongoerrors.Error] from the response. -func (r *ReplyResponse) CommandError() *mongoerrors.Error { - return r.err -} - // MsgHandlerFunc represents a function/method that processes a single OP_MSG command. // // The passed context is canceled when the client disconnects. diff --git a/website/docs/telemetry.md b/website/docs/telemetry.md index aa88bd00..594abfa1 100644 --- a/website/docs/telemetry.md +++ b/website/docs/telemetry.md @@ -119,7 +119,7 @@ When a FerretDB update is available, the telemetry service responds with information about the latest FerretDB version. This information is logged in server logs and available via the `getLog` command with the `startupWarnings` argument, making it visible when connecting with various tools such as `mongosh`. -While you may not upgrade to the latest release immediately, +While you may not update to the latest release immediately, ensure you update early to take advantage of recent bug fixes, new features, and performance improvements. ## Configuration