From a7b2580858a72b18e94261bb2425d1215fc2b9ce Mon Sep 17 00:00:00 2001 From: James Chao Date: Wed, 12 Aug 2026 23:03:40 -0700 Subject: [PATCH 1/2] Change the http status code from 500 to 504 when cassandra timeout --- db/cassandra/cassandra_client.go | 28 ++++ db/database_client.go | 3 + db/sqlite/sqlite_client.go | 4 + http/cassandra_timeout_test.go | 241 +++++++++++++++++++++++++++++++ http/document_handler.go | 28 ++-- http/multipart.go | 30 ++-- http/poke_handler.go | 6 +- http/refsubdocument_handler.go | 6 +- http/rootdocument_handler.go | 4 +- http/supplementary_handler.go | 4 +- http/webconfig_server.go | 9 ++ 11 files changed, 325 insertions(+), 38 deletions(-) create mode 100644 http/cassandra_timeout_test.go diff --git a/db/cassandra/cassandra_client.go b/db/cassandra/cassandra_client.go index 865647f..4c6f5da 100644 --- a/db/cassandra/cassandra_client.go +++ b/db/cassandra/cassandra_client.go @@ -18,6 +18,7 @@ package cassandra import ( + "context" "crypto/tls" "crypto/x509" "errors" @@ -343,6 +344,33 @@ func (c *CassandraClient) IsDbNotFound(err error) bool { return errors.Is(err, gocql.ErrNotFound) } +func (c *CassandraClient) IsDbTimeout(err error) bool { + if err == nil { + return false + } + if errors.Is(err, gocql.ErrTimeoutNoResponse) { + return true + } + if errors.Is(err, gocql.ErrConnectionClosed) { + return true + } + if errors.Is(err, context.DeadlineExceeded) { + return true + } + if errors.Is(err, context.Canceled) { + return true + } + var readTimeout *gocql.RequestErrReadTimeout + if errors.As(err, &readTimeout) { + return true + } + var writeTimeout *gocql.RequestErrWriteTimeout + if errors.As(err, &writeTimeout) { + return true + } + return false +} + func (c *CassandraClient) Close() error { c.Session.Close() return nil diff --git a/db/database_client.go b/db/database_client.go index 0548a81..f9d45ef 100644 --- a/db/database_client.go +++ b/db/database_client.go @@ -48,6 +48,9 @@ type DatabaseClient interface { // not found IsDbNotFound(error) bool + // timeout + IsDbTimeout(error) bool + // set metrics Metrics() *common.AppMetrics SetMetrics(*common.AppMetrics) diff --git a/db/sqlite/sqlite_client.go b/db/sqlite/sqlite_client.go index 7b1b5d9..c425057 100644 --- a/db/sqlite/sqlite_client.go +++ b/db/sqlite/sqlite_client.go @@ -130,6 +130,10 @@ func (c *SqliteClient) IsDbNotFound(err error) bool { return false } +func (c *SqliteClient) IsDbTimeout(err error) bool { + return false +} + func (c *SqliteClient) Metrics() *common.AppMetrics { return c.AppMetrics } diff --git a/http/cassandra_timeout_test.go b/http/cassandra_timeout_test.go new file mode 100644 index 0000000..a02a433 --- /dev/null +++ b/http/cassandra_timeout_test.go @@ -0,0 +1,241 @@ +/** +* Copyright 2021 Comcast Cable Communications Management, LLC +* +* 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. +* +* SPDX-License-Identifier: Apache-2.0 +*/ +package http + +import ( + "bytes" + "context" + "errors" + "fmt" + "net/http" + "testing" + + "github.com/gocql/gocql" + "github.com/prometheus/client_golang/prometheus" + "github.com/rdkcentral/webconfig/common" + "github.com/rdkcentral/webconfig/db" + "github.com/rdkcentral/webconfig/db/cassandra" + "github.com/rdkcentral/webconfig/util" + "gotest.tools/assert" +) + +// errSimulatedTimeout is a sentinel used by timeoutMockClient to trigger the 504 path. +var errSimulatedTimeout = errors.New("simulated cassandra timeout") + +// timeoutMockClient wraps the real (SQLite) DatabaseClient and overrides IsDbTimeout plus +// selected read methods to return errSimulatedTimeout, simulating a Cassandra timeout +// without a live Cassandra instance. Overridden methods: +// - IsDbTimeout — recognises errSimulatedTimeout as a timeout +// - GetSubDocument — used by GetSubDocumentHandler (GET /document/{id}) +// - GetRootDocumentLabels — used by PostSubDocumentHandler (POST /document/{id}) +// - GetRootDocument — used by BuildGetDocument (GET /config) +// - GetDocument — used by BuildGetDocument fallback paths +// +// All other interface methods (SetSubDocument, DeleteDocument, etc.) fall through to the +// embedded SQLite client and execute normally. +type timeoutMockClient struct { + db.DatabaseClient +} + +func (m *timeoutMockClient) IsDbTimeout(err error) bool { + return errors.Is(err, errSimulatedTimeout) +} + +func (m *timeoutMockClient) GetSubDocument(mac, subdocId string) (*common.SubDocument, error) { + return nil, errSimulatedTimeout +} + +func (m *timeoutMockClient) GetRootDocumentLabels(mac string) (prometheus.Labels, error) { + return nil, errSimulatedTimeout +} + +func (m *timeoutMockClient) GetRootDocument(mac string) (*common.RootDocument, error) { + return nil, errSimulatedTimeout +} + +func (m *timeoutMockClient) GetDocument(mac string, args ...interface{}) (*common.Document, error) { + return nil, errSimulatedTimeout +} + +// TestIsDbTimeout tests CassandraClient.IsDbTimeout against all gocql timeout error +// variants — including connection-closed and context deadline — as well as direct errors, +// wrapped errors, and multi-layer chains. Non-timeout errors must return false. +// No live Cassandra connection is required because IsDbTimeout is a pure error-inspection +// function. +func TestIsDbTimeout(t *testing.T) { + c := &cassandra.CassandraClient{} + + cases := []struct { + name string + err error + want bool + }{ + { + name: "ErrTimeoutNoResponse direct", + err: gocql.ErrTimeoutNoResponse, + want: true, + }, + { + name: "ErrTimeoutNoResponse wrapped once", + err: fmt.Errorf("layer: %w", gocql.ErrTimeoutNoResponse), + want: true, + }, + { + name: "ErrTimeoutNoResponse wrapped via common.NewError", + err: common.NewError(gocql.ErrTimeoutNoResponse), + want: true, + }, + { + name: "ErrTimeoutNoResponse wrapped multiple times", + err: fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", gocql.ErrTimeoutNoResponse)), + want: true, + }, + { + name: "RequestErrReadTimeout direct", + err: &gocql.RequestErrReadTimeout{}, + want: true, + }, + { + name: "RequestErrReadTimeout wrapped", + err: fmt.Errorf("layer: %w", &gocql.RequestErrReadTimeout{}), + want: true, + }, + { + name: "RequestErrWriteTimeout direct", + err: &gocql.RequestErrWriteTimeout{}, + want: true, + }, + { + name: "RequestErrWriteTimeout wrapped", + err: fmt.Errorf("layer: %w", &gocql.RequestErrWriteTimeout{}), + want: true, + }, + { + name: "ErrConnectionClosed direct", + err: gocql.ErrConnectionClosed, + want: true, + }, + { + name: "ErrConnectionClosed wrapped", + err: fmt.Errorf("layer: %w", gocql.ErrConnectionClosed), + want: true, + }, + { + name: "context.DeadlineExceeded direct", + err: context.DeadlineExceeded, + want: true, + }, + { + name: "context.DeadlineExceeded wrapped", + err: fmt.Errorf("layer: %w", context.DeadlineExceeded), + want: true, + }, + { + name: "context.Canceled direct", + err: context.Canceled, + want: true, + }, + { + name: "context.Canceled wrapped", + err: fmt.Errorf("layer: %w", context.Canceled), + want: true, + }, + { + name: "ErrNotFound is not a timeout", + err: gocql.ErrNotFound, + want: false, + }, + { + name: "generic error is not a timeout", + err: errors.New("some db error"), + want: false, + }, + { + name: "nil is not a timeout", + err: nil, + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := c.IsDbTimeout(tc.err) + assert.Equal(t, got, tc.want) + }) + } +} + +// TestDbErrToStatus verifies that dbErrToStatus maps a timeout error to 504 and a +// non-timeout error to 500, using the mock client to drive IsDbTimeout. +func TestDbErrToStatus(t *testing.T) { + server := NewWebconfigServer(sc, true) + server.DatabaseClient = &timeoutMockClient{DatabaseClient: server.DatabaseClient} + + assert.Equal(t, server.dbErrToStatus(errSimulatedTimeout), http.StatusGatewayTimeout) + assert.Equal(t, server.dbErrToStatus(errors.New("other error")), http.StatusInternalServerError) +} + +// TestGetSubDocumentHandlerCassandraTimeout verifies that GET /document/{id} returns 504 +// when the database layer reports a Cassandra timeout on GetSubDocument. +func TestGetSubDocumentHandlerCassandraTimeout(t *testing.T) { + server := NewWebconfigServer(sc, true) + server.DatabaseClient = &timeoutMockClient{DatabaseClient: server.DatabaseClient} + router := server.GetRouter(true) + + cpeMac := util.GenerateRandomCpeMac() + url := fmt.Sprintf("/api/v1/device/%v/document/lan", cpeMac) + req, err := http.NewRequest("GET", url, nil) + assert.NilError(t, err) + + res := ExecuteRequest(req, router).Result() + assert.Equal(t, res.StatusCode, http.StatusGatewayTimeout) +} + +// TestPostSubDocumentHandlerCassandraTimeout verifies that POST /document/{id} returns 504 +// when the database layer reports a Cassandra timeout on GetRootDocumentLabels. +func TestPostSubDocumentHandlerCassandraTimeout(t *testing.T) { + server := NewWebconfigServer(sc, true) + server.DatabaseClient = &timeoutMockClient{DatabaseClient: server.DatabaseClient} + router := server.GetRouter(true) + + cpeMac := util.GenerateRandomCpeMac() + url := fmt.Sprintf("/api/v1/device/%v/document/lan", cpeMac) + req, err := http.NewRequest("POST", url, bytes.NewReader([]byte{0x80})) + assert.NilError(t, err) + req.Header.Set(common.HeaderContentType, common.HeaderApplicationMsgpack) + + res := ExecuteRequest(req, router).Result() + assert.Equal(t, res.StatusCode, http.StatusGatewayTimeout) +} + +// TestMultipartConfigHandlerCassandraTimeout verifies that GET /config returns 504 when +// the database layer reports a Cassandra timeout during document retrieval. +func TestMultipartConfigHandlerCassandraTimeout(t *testing.T) { + server := NewWebconfigServer(sc, true) + server.DatabaseClient = &timeoutMockClient{DatabaseClient: server.DatabaseClient} + router := server.GetRouter(true) + + cpeMac := util.GenerateRandomCpeMac() + url := fmt.Sprintf("/api/v1/device/%v/config", cpeMac) + req, err := http.NewRequest("GET", url, nil) + assert.NilError(t, err) + req.Header.Set(common.HeaderSchemaVersion, "none") + + res := ExecuteRequest(req, router).Result() + assert.Equal(t, res.StatusCode, http.StatusGatewayTimeout) +} diff --git a/http/document_handler.go b/http/document_handler.go index e833c8a..210e15c 100644 --- a/http/document_handler.go +++ b/http/document_handler.go @@ -79,7 +79,7 @@ func (s *WebconfigServer) GetSubDocumentHandler(w http.ResponseWriter, r *http.R Error(w, http.StatusNotFound, nil) } else { LogError(w, err) - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } @@ -169,14 +169,14 @@ func (s *WebconfigServer) PostSubDocumentHandler(w http.ResponseWriter, r *http. labels, err := s.GetRootDocumentLabels(deviceId) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } labels["client"] = metricsAgent err = s.SetSubDocument(deviceId, subdocId, subdoc, oldState, labels, fields) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } @@ -187,7 +187,7 @@ func (s *WebconfigServer) PostSubDocumentHandler(w http.ResponseWriter, r *http. if s.IsDbNotFound(err) { doc = common.NewDocument(nil) } else { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } } @@ -196,7 +196,7 @@ func (s *WebconfigServer) PostSubDocumentHandler(w http.ResponseWriter, r *http. newRootVersion = db.HashRootVersion(doc.VersionMap()) err = s.SetRootDocumentVersion(deviceId, newRootVersion) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } rootVersionMap[deviceId] = newRootVersion @@ -233,7 +233,7 @@ func (s *WebconfigServer) DeleteSubDocumentHandler(w http.ResponseWriter, r *htt if s.IsDbNotFound(err) { Error(w, http.StatusNotFound, nil) } else { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } @@ -245,11 +245,12 @@ func (s *WebconfigServer) DeleteSubDocumentHandler(w http.ResponseWriter, r *htt if s.IsDbNotFound(err) { err := s.DeleteRootDocumentVersion(mac) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) + return } WriteOkResponse(w, nil) } else { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } @@ -257,7 +258,7 @@ func (s *WebconfigServer) DeleteSubDocumentHandler(w http.ResponseWriter, r *htt newRootVersion := db.HashRootVersion(doc.VersionMap()) err = s.SetRootDocumentVersion(mac, newRootVersion) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } @@ -290,7 +291,7 @@ func (s *WebconfigServer) DeleteDocumentHandler(w http.ResponseWriter, r *http.R if s.IsDbNotFound(err) { Error(w, http.StatusNotFound, nil) } else { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } @@ -302,11 +303,12 @@ func (s *WebconfigServer) DeleteDocumentHandler(w http.ResponseWriter, r *http.R if s.IsDbNotFound(err) { err := s.DeleteRootDocumentVersion(mac) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) + return } WriteOkResponse(w, nil) } else { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } @@ -314,7 +316,7 @@ func (s *WebconfigServer) DeleteDocumentHandler(w http.ResponseWriter, r *http.R newRootVersion := db.HashRootVersion(doc.VersionMap()) err = s.SetRootDocumentVersion(mac, newRootVersion) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } diff --git a/http/multipart.go b/http/multipart.go index bd154ba..d431139 100644 --- a/http/multipart.go +++ b/http/multipart.go @@ -147,7 +147,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin } if err != nil { if !s.IsDbNotFound(err) { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } return http.StatusNotFound, respHeader, nil, common.NewError(err) } @@ -164,7 +164,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin document, err = db.LoadRefSubDocuments(c, document, fields) if err != nil { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } if s.FilterOutputByBitmapEnabled() { @@ -183,7 +183,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin // skip updating states if userAgent != "mget" { if err := db.UpdateDocumentStateIndeployment(c, mac, document, fields); err != nil { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } } @@ -194,7 +194,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin if err != nil { if !s.IsDbNotFound(err) { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } // 404 if !postUpstream { @@ -216,7 +216,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin if !postUpstream { document, err = db.LoadRefSubDocuments(c, document, fields) if err != nil { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } } @@ -243,7 +243,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin if !postUpstream { // update states to InDeployment before the final response if err := db.UpdateDocumentStateIndeployment(c, mac, document, fields); err != nil { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } // 304 @@ -305,7 +305,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin if errors.As(err, &rherr) { return rherr.StatusCode, respHeader, nil, common.NewError(err) } - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } // ==== parse the upstreamRespBytes and store them ==== @@ -331,7 +331,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin // update states based on the final document err = db.WriteDocumentFromUpstream(c, mac, upstreamRespEtag, finalDocument, document, false, deviceVersionMap, fields) if err != nil { - return http.StatusInternalServerError, upstreamRespHeader, upstreamRespBytes, common.NewError(err) + return s.dbErrToStatus(err), upstreamRespHeader, upstreamRespBytes, common.NewError(err) } } @@ -351,7 +351,7 @@ func BuildWebconfigResponse(s *WebconfigServer, rHeader http.Header, route strin finalFilteredDocument, err = db.LoadRefSubDocuments(c, finalFilteredDocument, fields) if err != nil { - return http.StatusInternalServerError, upstreamRespHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), upstreamRespHeader, nil, common.NewError(err) } finalFilteredBytes, err := finalFilteredDocument.Bytes() if err != nil { @@ -375,7 +375,7 @@ func BuildFactoryResetResponse(s *WebconfigServer, rHeader http.Header, fields l rootDocument, err := db.PreprocessRootDocument(c, rHeader, mac, partnerId, fields) if err != nil { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } document, err := c.GetDocument(mac, fields) @@ -383,7 +383,7 @@ func BuildFactoryResetResponse(s *WebconfigServer, rHeader http.Header, fields l if s.IsDbNotFound(err) { return http.StatusNotFound, respHeader, nil, nil } else { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } } if document == nil { @@ -400,7 +400,7 @@ func BuildFactoryResetResponse(s *WebconfigServer, rHeader http.Header, fields l if !s.UpstreamEnabled() { err := c.DeleteDocument(mac) if err != nil { - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } return http.StatusNotFound, respHeader, nil, nil } @@ -437,7 +437,7 @@ func BuildFactoryResetResponse(s *WebconfigServer, rHeader http.Header, fields l if errors.As(err, &rherr) { return rherr.StatusCode, respHeader, nil, common.NewError(err) } - return http.StatusInternalServerError, respHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), respHeader, nil, common.NewError(err) } // ==== parse the upstreamRespBytes and store them ==== @@ -460,7 +460,7 @@ func BuildFactoryResetResponse(s *WebconfigServer, rHeader http.Header, fields l // update states based on the final document err = db.WriteDocumentFromUpstream(c, mac, upstreamRespEtag, finalDocument, document, true, nil, fields) if err != nil { - return http.StatusInternalServerError, upstreamRespHeader, upstreamRespBytes, common.NewError(err) + return s.dbErrToStatus(err), upstreamRespHeader, upstreamRespBytes, common.NewError(err) } } @@ -470,7 +470,7 @@ func BuildFactoryResetResponse(s *WebconfigServer, rHeader http.Header, fields l finalDocument, err = db.LoadRefSubDocuments(c, finalDocument, fields) if err != nil { - return http.StatusInternalServerError, upstreamRespHeader, nil, common.NewError(err) + return s.dbErrToStatus(err), upstreamRespHeader, nil, common.NewError(err) } // filter by bitmaps and blockedIds diff --git a/http/poke_handler.go b/http/poke_handler.go index 266023d..7ab2a5a 100644 --- a/http/poke_handler.go +++ b/http/poke_handler.go @@ -89,7 +89,7 @@ func (s *WebconfigServer) PokeHandler(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusNotFound, nil) return } - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } if document.Length() == 0 { @@ -118,7 +118,7 @@ func (s *WebconfigServer) PokeHandler(w http.ResponseWriter, r *http.Request) { err = db.UpdateStatesInBatch(s.DatabaseClient, deviceId, metricsAgent, fields, document.StateMap()) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } } @@ -134,7 +134,7 @@ func (s *WebconfigServer) PokeHandler(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusNoContent, nil) return } - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } if document.Length() == 0 { diff --git a/http/refsubdocument_handler.go b/http/refsubdocument_handler.go index cd8743d..1cd0ed0 100644 --- a/http/refsubdocument_handler.go +++ b/http/refsubdocument_handler.go @@ -48,7 +48,7 @@ func (s *WebconfigServer) GetRefSubDocumentHandler(w http.ResponseWriter, r *htt Error(w, http.StatusNotFound, nil) } else { LogError(w, err) - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } @@ -88,7 +88,7 @@ func (s *WebconfigServer) PostRefSubDocumentHandler(w http.ResponseWriter, r *ht err = s.SetRefSubDocument(refId, refsubdoc) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } @@ -117,7 +117,7 @@ func (s *WebconfigServer) DeleteRefSubDocumentHandler(w http.ResponseWriter, r * if s.IsDbNotFound(err) { Error(w, http.StatusNotFound, nil) } else { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) } return } diff --git a/http/rootdocument_handler.go b/http/rootdocument_handler.go index 2d4a5e3..8d3b396 100644 --- a/http/rootdocument_handler.go +++ b/http/rootdocument_handler.go @@ -48,7 +48,7 @@ func (s *WebconfigServer) GetRootDocumentHandler(w http.ResponseWriter, r *http. Error(w, http.StatusNotFound, nil) return } - Error(w, http.StatusInternalServerError, err) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } @@ -98,7 +98,7 @@ func (s *WebconfigServer) PostRootDocumentHandler(w http.ResponseWriter, r *http } err = s.SetRootDocument(mac, rootdoc) if err != nil { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } diff --git a/http/supplementary_handler.go b/http/supplementary_handler.go index 5561b85..079d7fe 100644 --- a/http/supplementary_handler.go +++ b/http/supplementary_handler.go @@ -59,7 +59,7 @@ func (s *WebconfigServer) MultipartSupplementaryHandler(w http.ResponseWriter, r // Check state from xpc_group_config by cpe_mac and group_id=telemetry telemetrySubdoc, err := s.GetSubDocument(mac, "telemetry") if err != nil && !s.IsDbNotFound(err) { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } @@ -156,7 +156,7 @@ func (s *WebconfigServer) MultipartSupplementaryHandler(w http.ResponseWriter, r rootdoc, err = s.GetRootDocument(mac) if err != nil { if !s.IsDbNotFound(err) { - Error(w, http.StatusInternalServerError, common.NewError(err)) + Error(w, s.dbErrToStatus(err), common.NewError(err)) return } } diff --git a/http/webconfig_server.go b/http/webconfig_server.go index dbbbe0b..5ba1d9f 100644 --- a/http/webconfig_server.go +++ b/http/webconfig_server.go @@ -1311,3 +1311,12 @@ func (s *WebconfigServer) SpanMiddleware(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +// dbErrToStatus maps a database error to its HTTP status code. +// Cassandra timeouts become 504; all other errors become 500. +func (s *WebconfigServer) dbErrToStatus(err error) int { + if s.IsDbTimeout(err) { + return http.StatusGatewayTimeout + } + return http.StatusInternalServerError +} From be83191c43cfe1cfe75542a18d5f32f44841c766 Mon Sep 17 00:00:00 2001 From: James Chao Date: Wed, 12 Aug 2026 23:31:18 -0700 Subject: [PATCH 2/2] context cancelled should not be considered as timeout-like error and return 504 --- db/cassandra/cassandra_client.go | 5 ++--- http/cassandra_timeout_test.go | 4 ++-- http/webconfig_server.go | 5 +++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/db/cassandra/cassandra_client.go b/db/cassandra/cassandra_client.go index 4c6f5da..6c7f6d7 100644 --- a/db/cassandra/cassandra_client.go +++ b/db/cassandra/cassandra_client.go @@ -357,9 +357,8 @@ func (c *CassandraClient) IsDbTimeout(err error) bool { if errors.Is(err, context.DeadlineExceeded) { return true } - if errors.Is(err, context.Canceled) { - return true - } + // context.Canceled is excluded: it indicates caller cancellation (client disconnect), + // not a DB/network timeout. Mapping it to 504 would misrepresent the failure cause. var readTimeout *gocql.RequestErrReadTimeout if errors.As(err, &readTimeout) { return true diff --git a/http/cassandra_timeout_test.go b/http/cassandra_timeout_test.go index a02a433..87b1b8f 100644 --- a/http/cassandra_timeout_test.go +++ b/http/cassandra_timeout_test.go @@ -148,12 +148,12 @@ func TestIsDbTimeout(t *testing.T) { { name: "context.Canceled direct", err: context.Canceled, - want: true, + want: false, }, { name: "context.Canceled wrapped", err: fmt.Errorf("layer: %w", context.Canceled), - want: true, + want: false, }, { name: "ErrNotFound is not a timeout", diff --git a/http/webconfig_server.go b/http/webconfig_server.go index 5ba1d9f..dcdaf30 100644 --- a/http/webconfig_server.go +++ b/http/webconfig_server.go @@ -1312,8 +1312,9 @@ func (s *WebconfigServer) SpanMiddleware(next http.Handler) http.Handler { }) } -// dbErrToStatus maps a database error to its HTTP status code. -// Cassandra timeouts become 504; all other errors become 500. +// dbErrToStatus maps a dependency error to its HTTP status code. +// Timeout-like errors (Cassandra timeouts, connection closed, deadline exceeded) become 504; +// all other errors become 500. func (s *WebconfigServer) dbErrToStatus(err error) int { if s.IsDbTimeout(err) { return http.StatusGatewayTimeout