From 450b5e741ed3e878e9b6cc0a9d468830d3c1ff6e Mon Sep 17 00:00:00 2001 From: Rajneesh Kumar <1109773+rajneesh2k10@users.noreply.github.com> Date: Tue, 16 Jun 2026 17:26:46 +0000 Subject: [PATCH] api: return 400 instead of 500 on duplicate changefeed creation ErrChangeFeedAlreadyExists was not in the httpBadRequestError registry, so the v2 CreateChangefeed handler returned HTTP 500 when a client posted a changefeed ID that already existed. A duplicate-create is a client-side condition and should be a 4xx. Adds the error to the existing bad-request registry, mirroring how ErrChangeFeedNotExists is already classified, and adds a TestErrorHandleMiddleware regression test. close #5427 --- api/middleware/middleware_test.go | 52 +++++++++++++++++++++++++++++++ pkg/api/util.go | 3 +- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/api/middleware/middleware_test.go b/api/middleware/middleware_test.go index f605bbe20d..48ecab88ff 100644 --- a/api/middleware/middleware_test.go +++ b/api/middleware/middleware_test.go @@ -24,6 +24,7 @@ import ( "github.com/gin-gonic/gin" "github.com/pingcap/kvproto/pkg/keyspacepb" + cerrors "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/node" "github.com/pingcap/ticdc/pkg/pdutil" "github.com/pingcap/ticdc/pkg/server" @@ -205,6 +206,57 @@ func TestForwardToCoordinator(t *testing.T) { } } +func TestErrorHandleMiddleware(t *testing.T) { + gin.SetMode(gin.TestMode) + + cases := []struct { + name string + err error + wantStatus int + }{ + { + name: "no error returns 200", + err: nil, + wantStatus: http.StatusOK, + }, + { + name: "registered bad-request error returns 400", + err: cerrors.ErrAPIInvalidParam.GenWithStackByArgs("bad input"), + wantStatus: http.StatusBadRequest, + }, + { + name: "duplicate changefeed returns 400", + err: cerrors.ErrChangeFeedAlreadyExists.GenWithStackByArgs("foo"), + wantStatus: http.StatusBadRequest, + }, + { + name: "unclassified error returns 500", + err: errors.New("boom"), + wantStatus: http.StatusInternalServerError, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + router := gin.New() + router.Use(ErrorHandleMiddleware()) + router.GET("/test", func(c *gin.Context) { + if tc.err != nil { + _ = c.Error(tc.err) + return + } + c.Status(http.StatusOK) + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/test", nil) + router.ServeHTTP(w, req) + + require.Equal(t, tc.wantStatus, w.Code) + }) + } +} + // mockServer implements server.Server interface for testing type mockServer struct { server.Server diff --git a/pkg/api/util.go b/pkg/api/util.go index 465b8a279d..95342ce7ae 100644 --- a/pkg/api/util.go +++ b/pkg/api/util.go @@ -67,7 +67,8 @@ func NewHTTPError(err error) HTTPError { // httpBadRequestError is some errors that will cause a BadRequestError in http handler var httpBadRequestError = []*errors.Error{ cerror.ErrAPIInvalidParam, cerror.ErrSinkURIInvalid, cerror.ErrStartTsBeforeGC, - cerror.ErrChangeFeedNotExists, cerror.ErrTargetTsBeforeStartTs, cerror.ErrTableIneligible, + cerror.ErrChangeFeedNotExists, cerror.ErrChangeFeedAlreadyExists, + cerror.ErrTargetTsBeforeStartTs, cerror.ErrTableIneligible, cerror.ErrFilterRuleInvalid, cerror.ErrChangefeedUpdateRefused, cerror.ErrMySQLConnectionError, cerror.ErrMySQLInvalidConfig, cerror.ErrCaptureNotExist, cerror.ErrSchedulerRequestFailed, cerror.ErrActiveActiveTSOIndexIncompatible,