mirror of
https://github.com/karust/openserp.git
synced 2026-08-13 04:13:27 +08:00
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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{}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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))
|
||||
|
||||
71
core/server_panic_test.go
Normal file
71
core/server_panic_test.go
Normal file
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user