From 7dbff18e652d6fc3f82cd519e84852ad5c70d21c Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb Date: Mon, 31 Aug 2026 19:48:21 +0500 Subject: [PATCH] fix(chromium): fail fast with 503 when Chromium crashes (#1641) --- pkg/modules/chromium/browser.go | 37 ++++++++++++++ pkg/modules/chromium/chromium.go | 6 +++ pkg/modules/chromium/errortype_test.go | 1 + pkg/modules/chromium/events.go | 33 ++++++++++++ pkg/modules/chromium/routes.go | 10 ++++ pkg/modules/chromium/routes_test.go | 51 +++++++++++++++++++ .../features/chromium_convert_url.feature | 16 ++++++ 7 files changed, 154 insertions(+) create mode 100644 pkg/modules/chromium/routes_test.go diff --git a/pkg/modules/chromium/browser.go b/pkg/modules/chromium/browser.go index d0be247a..67eaa4f1 100644 --- a/pkg/modules/chromium/browser.go +++ b/pkg/modules/chromium/browser.go @@ -523,8 +523,45 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin cancelOnMainPageError: taskCancel, }) + var ( + crashed error + crashedMu sync.RWMutex + ) + + // See https://github.com/gotenberg/gotenberg/issues/1640. + listenForEventTargetCrashed(taskCtx, logger, eventTargetCrashedOptions{ + crashed: &crashed, + crashedMu: &crashedMu, + cancel: taskCancel, + }) + runErr := chromedp.Run(taskCtx, tasks...) + // A crashed renderer is the root cause of every other failure this + // conversion may have recorded, so check it first. + // See https://github.com/gotenberg/gotenberg/issues/1640. + crashedMu.RLock() + defer crashedMu.RUnlock() + + if crashed != nil { + return fmt.Errorf("handle tasks: %w", crashed) + } + + // The browser context is only ever canceled when the browser process + // dies or is stopped, never on a request timeout. If the run failed + // and the browser context is done, the conversion failed because the + // browser went away mid-flight; fail fast with the same crash error + // instead of letting the error fall through as a generic context + // cancellation. The check is gated on runErr so a successful + // conversion is never discarded by a browser death that lands right + // after it. + // See https://github.com/gotenberg/gotenberg/issues/1640. + if runErr != nil { + if err := b.ctx.Err(); err != nil { + return fmt.Errorf("handle tasks: %w", ErrChromiumCrashed) + } + } + // Check event-driven errors first — they take priority over chromedp.Run // errors because they carry the actual root cause (e.g., HTTP 500 from // the main page). When we cancel taskCtx on a main page error, diff --git a/pkg/modules/chromium/chromium.go b/pkg/modules/chromium/chromium.go index aa80b6cc..51173248 100644 --- a/pkg/modules/chromium/chromium.go +++ b/pkg/modules/chromium/chromium.go @@ -71,6 +71,10 @@ var ( // ErrResourceLoadingFailed happens when one or more resources failed to load. ErrResourceLoadingFailed = errors.New("resource loading failed") + // ErrChromiumCrashed happens when the Chromium renderer crashes during a + // conversion. + ErrChromiumCrashed = errors.New("chromium crashed") + // PDF specific. // ErrOmitBackgroundWithoutPrintBackground happens if @@ -1106,6 +1110,8 @@ func chromiumErrorType(err error, queueReason string) string { errors.Is(err, ErrInvalidEvaluationExpression), errors.Is(err, ErrInvalidSelectorQuery): return gotenberg.ErrorTypeInvalidInput + case errors.Is(err, ErrChromiumCrashed): + return "chromium_unavailable" case errors.Is(err, gotenberg.ErrMaximumQueueSizeExceeded): return queueReason case errors.Is(err, gotenberg.ErrProcessAlreadyRestarting): diff --git a/pkg/modules/chromium/errortype_test.go b/pkg/modules/chromium/errortype_test.go index 113361d8..9b7fc0bb 100644 --- a/pkg/modules/chromium/errortype_test.go +++ b/pkg/modules/chromium/errortype_test.go @@ -21,6 +21,7 @@ func TestChromiumErrorType(t *testing.T) { {"invalid resource http status", ErrInvalidResourceHttpStatusCode, "chromium_unavailable", "invalid_input"}, {"loading failed", ErrLoadingFailed, "chromium_unavailable", "invalid_input"}, {"resource loading failed", ErrResourceLoadingFailed, "chromium_unavailable", "invalid_input"}, + {"crashed", ErrChromiumCrashed, "chromium_unavailable", "chromium_unavailable"}, {"invalid evaluation expression", ErrInvalidEvaluationExpression, "chromium_unavailable", "invalid_input"}, {"invalid selector query", ErrInvalidSelectorQuery, "chromium_unavailable", "invalid_input"}, {"pdf queue", gotenberg.ErrMaximumQueueSizeExceeded, "chromium_unavailable", "chromium_unavailable"}, diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index b85beba2..ba2a7305 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -14,6 +14,7 @@ import ( "github.com/chromedp/cdproto/cdp" "github.com/chromedp/cdproto/fetch" + "github.com/chromedp/cdproto/inspector" "github.com/chromedp/cdproto/network" "github.com/chromedp/cdproto/page" "github.com/chromedp/cdproto/runtime" @@ -521,6 +522,38 @@ func listenForEventExceptionThrown(ctx context.Context, logger *slog.Logger, con }) } +type eventTargetCrashedOptions struct { + crashed *error + crashedMu *sync.RWMutex + cancel context.CancelFunc +} + +// listenForEventTargetCrashed listens for the Inspector.targetCrashed event, +// which Chromium sends when the renderer serving the conversion's tab +// crashes. chromedp enables the Inspector domain on every target but does +// not handle this event: left alone, the in-flight CDP command never +// receives a response and the conversion blocks until the request deadline. +// Record the crash and cancel the task context so the conversion fails fast +// instead. +// See https://github.com/gotenberg/gotenberg/issues/1640. +func listenForEventTargetCrashed(ctx context.Context, logger *slog.Logger, options eventTargetCrashedOptions) { + chromedp.ListenTarget(ctx, func(ev any) { + if _, ok := ev.(*inspector.EventTargetCrashed); ok { + logger.DebugContext(ctx, "event EventTargetCrashed fired") + + options.crashedMu.Lock() + defer options.crashedMu.Unlock() + + *options.crashed = ErrChromiumCrashed + + // Cancel the task context so the in-flight CDP command aborts + // immediately instead of waiting for a response the crashed + // renderer can never send. + options.cancel() + } + }) +} + // waitForEventDomContentEventFired registers a listener for the // DomContentEventFired event and returns a waiter that blocks until the // event fires or ctx is done. The listener registers at call time, not diff --git a/pkg/modules/chromium/routes.go b/pkg/modules/chromium/routes.go index 92a21fc5..705935d5 100644 --- a/pkg/modules/chromium/routes.go +++ b/pkg/modules/chromium/routes.go @@ -1020,6 +1020,16 @@ func handleChromiumError(err error, options Options) error { return nil } + if errors.Is(err, ErrChromiumCrashed) { + return api.WrapError( + err, + api.NewSentinelHttpError( + http.StatusServiceUnavailable, + "Chromium crashed while processing the request. Retry, or reduce the workload if the problem persists.", + ), + ) + } + if errors.Is(err, ErrInvalidEvaluationExpression) { if options.WaitForExpression == "" { // We do not expect the 'waitWindowStatus' form field to return diff --git a/pkg/modules/chromium/routes_test.go b/pkg/modules/chromium/routes_test.go new file mode 100644 index 00000000..12d4a25c --- /dev/null +++ b/pkg/modules/chromium/routes_test.go @@ -0,0 +1,51 @@ +package chromium + +import ( + "fmt" + "net/http" + "testing" + + "github.com/gotenberg/gotenberg/v8/pkg/modules/api" +) + +// TestHandleChromiumError_Crashed pins the mapping of a Chromium renderer +// crash to a 503 Service Unavailable. When the renderer crashes mid-conversion, +// the request must fail fast with 503 rather than hang until the deadline and +// surface as a generic timeout. +// See https://github.com/gotenberg/gotenberg/issues/1640. +func TestHandleChromiumError_Crashed(t *testing.T) { + // Mirror the wrapping done by [chromiumBrowser.do]. + err := handleChromiumError(fmt.Errorf("handle tasks: %w", ErrChromiumCrashed), Options{}) + if err == nil { + t.Fatal("expected an error, got none") + } + + status, message := api.ParseError(err) + if status != http.StatusServiceUnavailable { + t.Errorf("status = %d, want %d (message: %s)", status, http.StatusServiceUnavailable, message) + } + + want := "Chromium crashed while processing the request. Retry, or reduce the workload if the problem persists." + if message != want { + t.Errorf("message = %q, want %q", message, want) + } +} + +// TestHandleChromiumError_CrashedTakesPrecedence guards the ordering in +// [handleChromiumError]: a crash is a server-side failure and must map to 503 +// even when the error chain also carries a marker that another branch would +// map to a client-error status. +func TestHandleChromiumError_CrashedTakesPrecedence(t *testing.T) { + err := handleChromiumError( + fmt.Errorf("handle tasks: %w; %w", ErrChromiumCrashed, ErrInvalidHttpStatusCode), + Options{}, + ) + if err == nil { + t.Fatal("expected an error, got none") + } + + status, _ := api.ParseError(err) + if status != http.StatusServiceUnavailable { + t.Errorf("status = %d, want %d", status, http.StatusServiceUnavailable) + } +} diff --git a/test/integration/features/chromium_convert_url.feature b/test/integration/features/chromium_convert_url.feature index a8020d49..7e6a5bec 100644 --- a/test/integration/features/chromium_convert_url.feature +++ b/test/integration/features/chromium_convert_url.feature @@ -1401,3 +1401,19 @@ Feature: /forms/chromium/convert/url Then the response header "Content-Type" should be "application/pdf" Then there should be 1 PDF(s) in the response Then the "foo.pdf" PDF should have 1 page(s) + + # chrome://crash makes the renderer crash deterministically, the same + # failure class as a renderer crash triggered by the page content. The + # request must fail fast with a 503 instead of hanging until the API + # timeout. + # See https://github.com/gotenberg/gotenberg/issues/1640. + Scenario: POST /forms/chromium/convert/url (Chromium crash fails fast with 503) + Given I have a default Gotenberg container + When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s): + | url | chrome://crash | field | + Then the response status code should be 503 + Then the response header "Content-Type" should be "text/plain; charset=UTF-8" + Then the response body should contain string: + """ + Chromium crashed while processing the request + """