From d20a7c53cd860ef11f4f2ed2a0c95fb5eab84f97 Mon Sep 17 00:00:00 2001 From: Rustem Kamalov Date: Fri, 12 Jun 2026 04:24:06 +0300 Subject: [PATCH] fix: centralize engine panic recovery in resilient layer A rod panic during SearchImage propagated uncaught into fasthttp and killed the whole process: per-engine recover blocks only covered Search (5 of 6 engines had no recovery on SearchImage), and there was no Fiber recover middleware. Regression test: a panicking SearchImage returns 502 engine_internal and the server keeps serving. --- baidu/search.go | 6 ---- baidu/search_raw.go | 6 ---- bing/search.go | 6 ---- core/resilient.go | 27 ++++++++++----- core/server.go | 5 +++ core/server_panic_test.go | 71 +++++++++++++++++++++++++++++++++++++++ duckduckgo/search.go | 6 ---- ecosia/search.go | 14 -------- ecosia/search_raw.go | 6 ---- google/search.go | 7 ---- google/search_raw.go | 6 ---- yandex/search.go | 6 ---- yandex/search_raw.go | 6 ---- 13 files changed, 94 insertions(+), 78 deletions(-) create mode 100644 core/server_panic_test.go diff --git a/baidu/search.go b/baidu/search.go index 69f2b92..503e19e 100644 --- a/baidu/search.go +++ b/baidu/search.go @@ -87,12 +87,6 @@ func (baid *Baidu) Search(ctx context.Context, query core.Query) (results []core baid = &scoped baid.logger.Debug("Starting search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, baid.Name(), recovered, baid.logger) - results = nil - } - }() // Build URL from query struct to open in browser url, err := BuildURL(query) diff --git a/baidu/search_raw.go b/baidu/search_raw.go index 26afdb0..d3a365b 100644 --- a/baidu/search_raw.go +++ b/baidu/search_raw.go @@ -26,12 +26,6 @@ func classifyBaiduRawHTML(body []byte) error { func Search(ctx context.Context, query core.Query) (results []core.SearchResult, err error) { ctx = core.PrepareEngineContext(ctx, query, "baidu", false) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, "baidu", recovered, nil) - results = nil - } - }() searchURL, err := BuildURL(query) if err != nil { diff --git a/bing/search.go b/bing/search.go index c69a092..f519abe 100644 --- a/bing/search.go +++ b/bing/search.go @@ -151,12 +151,6 @@ func (bing *Bing) Search(ctx context.Context, query core.Query) (results []core. bing = &scoped bing.logger.Debug("Starting search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, bing.Name(), recovered, bing.logger) - results = nil - } - }() searchResults := []core.SearchResult{} diff --git a/core/resilient.go b/core/resilient.go index 8de16ba..6ba0ecb 100644 --- a/core/resilient.go +++ b/core/resilient.go @@ -202,16 +202,8 @@ func (rs *ResilientSearcher) searchWithProtection(ctx context.Context, engine Se attemptMeta.Used = MaskProxyURL(proxyURL) } - var ( - results []SearchResult - err error - ) requestCtx := proxyRequestContext(callCtx, engine.Name(), attemptQuery) - if isImage { - results, err = engine.SearchImage(requestCtx, attemptQuery) - } else { - results, err = engine.Search(requestCtx, attemptQuery) - } + results, err := invokeEngine(requestCtx, engine, attemptQuery, isImage) if reportToRegistry { rs.reportProxyAttempt(engineCtx, proxyURL, err) @@ -241,6 +233,23 @@ func (rs *ResilientSearcher) searchWithProtection(ctx context.Context, engine Se return result.Results, attemptMeta, nil } +// invokeEngine is the single panic-recovery point for every engine call made +// through the resilient pipeline (browser, raw, and any future engine method). +// A rod/CDP panic surfaces as ErrEngineInternal instead of killing the process. +func invokeEngine(ctx context.Context, engine SearchEngine, q Query, isImage bool) (results []SearchResult, err error) { + defer func() { + if recovered := recover(); recovered != nil { + results = nil + err = RecoverEnginePanicWithContext(ctx, engine.Name(), recovered, nil) + } + }() + + if isImage { + return engine.SearchImage(ctx, q) + } + return engine.Search(ctx, q) +} + // SearchAllParallel applies retry/circuit protections per engine for mega search. // Returns results, list of engines that responded, and list of engines that failed. func (rs *ResilientSearcher) SearchAllParallel(ctx context.Context, q Query, engines []SearchEngine) ([]MegaSearchResult, []string, []string) { diff --git a/core/server.go b/core/server.go index c4dec82..271e968 100644 --- a/core/server.go +++ b/core/server.go @@ -18,6 +18,7 @@ import ( "time" "github.com/gofiber/fiber/v2" + fiberrecover "github.com/gofiber/fiber/v2/middleware/recover" browserprofile "github.com/karust/openserp/core/browser" "github.com/karust/openserp/core/fpcheck" "github.com/karust/openserp/core/fpcheck/detectors" @@ -155,6 +156,10 @@ func NewServerWithOptions(host string, port int, opts ServerOptions, searchEngin }).Info("Response cache enabled") } + // Defense-in-depth: engine panics are recovered in the resilient layer + // (invokeEngine); this catches panics in handlers that bypass it (parse, + // extract, stats) so the process survives. + app.Use(fiberrecover.New(fiberrecover.Config{EnableStackTrace: true})) app.Use(RequestContextMiddleware()) if opts.EnableCORS { app.Use(CORSMiddleware(opts.CORS)) diff --git a/core/server_panic_test.go b/core/server_panic_test.go new file mode 100644 index 0000000..070493e --- /dev/null +++ b/core/server_panic_test.go @@ -0,0 +1,71 @@ +package core + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" +) + +// FP-1: panics from engine code must be converted to ErrEngineInternal by the +// central recovery in the resilient layer (invokeEngine) instead of killing +// the process. SearchImage had no per-engine recovery in 5 of 6 engines. +func TestInvokeEngineRecoversPanics(t *testing.T) { + engine := &engineMock{ + name: "google", + initialized: true, + searchFn: func(_ context.Context, _ Query) ([]SearchResult, error) { + panic("rod: page crashed") + }, + imageFn: func(_ context.Context, _ Query) ([]SearchResult, error) { + panic("rod: object not found") + }, + } + + for _, isImage := range []bool{false, true} { + results, err := invokeEngine(context.Background(), engine, Query{Text: "golang"}, isImage) + if results != nil { + t.Fatalf("isImage=%v: expected nil results after panic, got %v", isImage, results) + } + if !errors.Is(err, ErrEngineInternal) { + t.Fatalf("isImage=%v: expected ErrEngineInternal, got %v", isImage, err) + } + } +} + +func TestPanickingSearchImageReturns502AndServerSurvives(t *testing.T) { + engine := &engineMock{ + name: "google", + initialized: true, + imageFn: func(_ context.Context, _ Query) ([]SearchResult, error) { + panic("rod: page crashed") + }, + } + + opts := DefaultServerOptions() + opts.Resilience.Retry.MaxRetries = 0 + srv := NewServerWithOptions("127.0.0.1", 7130, opts, engine) + + req := httptest.NewRequest(http.MethodGet, "/google/image?text=golang", nil) + resp, err := srv.app.Test(req, -1) + if err != nil { + t.Fatalf("image request failed: %v", err) + } + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("expected 502 for panicking SearchImage, got %d", resp.StatusCode) + } + var payload JSONErrorResponse + if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil { + t.Fatalf("decode error response: %v", err) + } + if payload.Error != "engine_internal" { + t.Fatalf("expected error=engine_internal, got %q", payload.Error) + } + + second := request(t, srv, "/google/search?text=golang") + if second.StatusCode != http.StatusOK { + t.Fatalf("expected server to keep serving after panic, got %d", second.StatusCode) + } +} diff --git a/duckduckgo/search.go b/duckduckgo/search.go index b0e8bfd..43a73c9 100644 --- a/duckduckgo/search.go +++ b/duckduckgo/search.go @@ -166,12 +166,6 @@ func (ddg *DuckDuckGo) Search(ctx context.Context, query core.Query) (results [] ddg = &scoped ddg.logger.Debug("Starting search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, ddg.Name(), recovered, ddg.logger) - results = nil - } - }() allResults := []core.SearchResult{} var pageFeatures []core.SerpFeature diff --git a/ecosia/search.go b/ecosia/search.go index 0cbb2d3..ce6c38a 100644 --- a/ecosia/search.go +++ b/ecosia/search.go @@ -131,13 +131,6 @@ func (e *Ecosia) Search(ctx context.Context, query core.Query) (results []core.S e = &scoped e.logger.Debug("Starting search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, e.Name(), recovered, e.logger) - results = nil - } - }() - // nextRank counts up across pages for organic results; nextAdRank counts // up within sponsored results so ad rank stays separate from SEO rank. all := []core.SearchResult{} @@ -288,13 +281,6 @@ func (e *Ecosia) SearchImage(ctx context.Context, query core.Query) (results []c e = &scoped e.logger.Debug("Starting image search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, e.Name(), recovered, e.logger) - results = nil - } - }() - out := []core.SearchResult{} pageNum := 0 nextRank := 1 diff --git a/ecosia/search_raw.go b/ecosia/search_raw.go index c1d8de3..774c9de 100644 --- a/ecosia/search_raw.go +++ b/ecosia/search_raw.go @@ -72,12 +72,6 @@ func imageResultParser(response *http.Response) ([]core.SearchResult, error) { func Search(ctx context.Context, query core.Query) (results []core.SearchResult, err error) { ctx = core.PrepareEngineContext(ctx, query, "ecosia", false) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, "ecosia", recovered, nil) - results = nil - } - }() pageNum, startRank, err := startPage(query.Start) if err != nil { diff --git a/google/search.go b/google/search.go index 3733393..4b97267 100644 --- a/google/search.go +++ b/google/search.go @@ -189,13 +189,6 @@ func (gogl *Google) Search(ctx context.Context, query core.Query) (results []cor gogl = &scoped gogl.logger.Debug("Starting search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, gogl.Name(), recovered, gogl.logger) - results = nil - } - }() - searchResults := []core.SearchResult{} // Build URL from query struct to open in browser diff --git a/google/search_raw.go b/google/search_raw.go index 6950bbd..bd1b717 100644 --- a/google/search_raw.go +++ b/google/search_raw.go @@ -128,12 +128,6 @@ func classifyGoogleRawHTML(body []byte) error { func Search(ctx context.Context, query core.Query) (results []core.SearchResult, err error) { ctx = core.PrepareEngineContext(ctx, query, "google", false) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, "google", recovered, nil) - results = nil - } - }() googleURL, err := BuildURL(query) if err != nil { diff --git a/yandex/search.go b/yandex/search.go index f6b9c42..5ea406b 100644 --- a/yandex/search.go +++ b/yandex/search.go @@ -185,12 +185,6 @@ func (yand *Yandex) Search(ctx context.Context, query core.Query) (results []cor yand = &scoped yand.logger.Debug("Starting search, query: %+v", query) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, yand.Name(), recovered, yand.logger) - results = nil - } - }() if query.Start < 0 { return nil, fmt.Errorf("incorrect start provided") } diff --git a/yandex/search_raw.go b/yandex/search_raw.go index 9af173c..3195226 100644 --- a/yandex/search_raw.go +++ b/yandex/search_raw.go @@ -26,12 +26,6 @@ func classifyYandexRawHTML(body []byte) error { func Search(ctx context.Context, query core.Query) (results []core.SearchResult, err error) { ctx = core.PrepareEngineContext(ctx, query, "yandex", false) - defer func() { - if recovered := recover(); recovered != nil { - err = core.RecoverEnginePanicWithContext(ctx, "yandex", recovered, nil) - results = nil - } - }() startPage, skipOnFirstPage, err := core.ComputePagination(query.Start, 10) if err != nil {