From 0b33be17a4f04bfd66071c6cdc67c243aa851cd8 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Fri, 27 Mar 2026 17:32:27 +0100 Subject: [PATCH] fix(chromium): waitForSelector and waitForExpression poll indefinitely when the main page returns a bad HTTP status (e.g., 500) --- pkg/modules/chromium/browser.go | 69 +++++++++++++++++++-------------- pkg/modules/chromium/events.go | 17 ++++++++ 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/pkg/modules/chromium/browser.go b/pkg/modules/chromium/browser.go index 57395d9d..cc758180 100644 --- a/pkg/modules/chromium/browser.go +++ b/pkg/modules/chromium/browser.go @@ -382,6 +382,7 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin ignoreResourceHttpStatusDomains: options.IgnoreResourceHttpStatusDomains, invalidResourceHttpStatusCode: &invalidResourceHttpStatusCode, invalidResourceHttpStatusCodeMu: &invalidResourceHttpStatusCodeMu, + cancelOnMainPageError: taskCancel, }) } @@ -411,11 +412,45 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin loadingFailedMu: &loadingFailedMu, resourceLoadingFailed: &resourceLoadingFailed, resourceLoadingFailedMu: &resourceLoadingFailedMu, + cancelOnMainPageError: taskCancel, }) - err = chromedp.Run(taskCtx, tasks...) - if err != nil { - errMessage := err.Error() + runErr := chromedp.Run(taskCtx, tasks...) + + // 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, + // chromedp.Run returns a context error that is less informative. + // See https://github.com/gotenberg/gotenberg/issues/1492. + + // See https://github.com/gotenberg/gotenberg/issues/613. + invalidHttpStatusCodeMu.RLock() + defer invalidHttpStatusCodeMu.RUnlock() + + if invalidHttpStatusCode != nil { + return fmt.Errorf("%v: %w", invalidHttpStatusCode, ErrInvalidHttpStatusCode) + } + + // See https://github.com/gotenberg/gotenberg/issues/1021. + invalidResourceHttpStatusCodeMu.RLock() + defer invalidResourceHttpStatusCodeMu.RUnlock() + + if invalidResourceHttpStatusCode != nil { + return fmt.Errorf("%v: %w", invalidResourceHttpStatusCode, ErrInvalidResourceHttpStatusCode) + } + + // See: + // https://github.com/gotenberg/gotenberg/issues/913. + // https://github.com/gotenberg/gotenberg/issues/959. + loadingFailedMu.RLock() + defer loadingFailedMu.RUnlock() + + if loadingFailed != nil { + return fmt.Errorf("%v: %w", loadingFailed, ErrLoadingFailed) + } + + if runErr != nil { + errMessage := runErr.Error() if strings.Contains(errMessage, "Printing failed (-32000)") { return ErrPrintingFailed @@ -437,23 +472,7 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin return ErrRpccMessageTooLarge } - return fmt.Errorf("handle tasks: %w", err) - } - - // See https://github.com/gotenberg/gotenberg/issues/613. - invalidHttpStatusCodeMu.RLock() - defer invalidHttpStatusCodeMu.RUnlock() - - if invalidHttpStatusCode != nil { - return fmt.Errorf("%v: %w", invalidHttpStatusCode, ErrInvalidHttpStatusCode) - } - - // See https://github.com/gotenberg/gotenberg/issues/1021. - invalidResourceHttpStatusCodeMu.RLock() - defer invalidResourceHttpStatusCodeMu.RUnlock() - - if invalidResourceHttpStatusCode != nil { - return fmt.Errorf("%v: %w", invalidResourceHttpStatusCode, ErrInvalidResourceHttpStatusCode) + return fmt.Errorf("handle tasks: %w", runErr) } // See https://github.com/gotenberg/gotenberg/issues/262. @@ -464,16 +483,6 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin return fmt.Errorf("%v: %w", consoleExceptions, ErrConsoleExceptions) } - // See: - // https://github.com/gotenberg/gotenberg/issues/913. - // https://github.com/gotenberg/gotenberg/issues/959. - loadingFailedMu.RLock() - defer loadingFailedMu.RUnlock() - - if loadingFailed != nil { - return fmt.Errorf("%v: %w", loadingFailed, ErrLoadingFailed) - } - // See https://github.com/gotenberg/gotenberg/issues/1021. if options.FailOnResourceLoadingFailed { if resourceLoadingFailed != nil { diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index 7773f4d2..5497a976 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -168,6 +168,7 @@ type eventResponseReceivedOptions struct { ignoreResourceHttpStatusDomains []string invalidResourceHttpStatusCode *error invalidResourceHttpStatusCodeMu *sync.RWMutex + cancelOnMainPageError context.CancelFunc } // listenForEventResponseReceived listens for an invalid HTTP status code @@ -206,6 +207,14 @@ func listenForEventResponseReceived( defer options.invalidHttpStatusCodeMu.Unlock() *options.invalidHttpStatusCode = fmt.Errorf("%d: %s", ev.Response.Status, ev.Response.StatusText) + + // Cancel the task context so that any in-flight wait + // operations (waitForSelector, waitForExpression, etc.) + // abort immediately instead of polling until timeout. + // See https://github.com/gotenberg/gotenberg/issues/1492. + if options.cancelOnMainPageError != nil { + options.cancelOnMainPageError() + } } return @@ -309,6 +318,7 @@ type eventLoadingFailedOptions struct { loadingFailedMu *sync.RWMutex resourceLoadingFailed *error resourceLoadingFailedMu *sync.RWMutex + cancelOnMainPageError context.CancelFunc } // listenForEventLoadingFailed listens for an event indicating that the main @@ -353,6 +363,13 @@ func listenForEventLoadingFailed(ctx context.Context, logger *slog.Logger, optio *options.loadingFailed = fmt.Errorf("%s", ev.ErrorText) + // Cancel the task context so that any in-flight wait + // operations abort immediately. + // See https://github.com/gotenberg/gotenberg/issues/1492. + if options.cancelOnMainPageError != nil { + options.cancelOnMainPageError() + } + return }