fix(chromium): increase the scope of loading failed errors (#962)

2026-08-08 00:22:14 +01:00 · 2024-09-07 15:54:01 +02:00
parent 300e73f07d
commit a3647fea8a
6 changed files with 60 additions and 31 deletions
--- a/pkg/modules/chromium/browser.go
+++ b/pkg/modules/chromium/browser.go
@@ -315,12 +315,14 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *zap.Logger, url string
 	}

 	var (
-		connectionRefused   error
-		connectionRefusedMu sync.RWMutex
+		loadingFailed   error
+		loadingFailedMu sync.RWMutex
 	)

-	// See https://github.com/gotenberg/gotenberg/issues/913.
-	listenForEventLoadingFailedOnConnectionRefused(taskCtx, logger, &connectionRefused, &connectionRefusedMu)
+	// See:
+	// https://github.com/gotenberg/gotenberg/issues/913.
+	// https://github.com/gotenberg/gotenberg/issues/959.
+	listenForEventLoadingFailed(taskCtx, logger, &loadingFailed, &loadingFailedMu)

 	err = chromedp.Run(taskCtx, tasks...)
 	if err != nil {
@@ -357,12 +359,14 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *zap.Logger, url string
 		return fmt.Errorf("%v: %w", consoleExceptions, ErrConsoleExceptions)
 	}

-	// See https://github.com/gotenberg/gotenberg/issues/913.
-	connectionRefusedMu.RLock()
-	defer connectionRefusedMu.RUnlock()
+	// See:
+	// https://github.com/gotenberg/gotenberg/issues/913.
+	// https://github.com/gotenberg/gotenberg/issues/959.
+	loadingFailedMu.RLock()
+	defer loadingFailedMu.RUnlock()

-	if connectionRefused != nil {
-		return fmt.Errorf("%v: %w", connectionRefused, ErrConnectionRefused)
+	if loadingFailed != nil {
+		return fmt.Errorf("%v: %w", loadingFailed, ErrLoadingFailed)
 	}

 	return nil
--- a/pkg/modules/chromium/browser_test.go
+++ b/pkg/modules/chromium/browser_test.go
@@ -480,7 +480,7 @@ func TestChromiumBrowser_pdf(t *testing.T) {
 			expectedError: ErrConsoleExceptions,
 		},
 		{
-			scenario: "ErrConnectionRefused",
+			scenario: "ErrLoadingFailed",
 			browser: newChromiumBrowser(
 				browserArguments{
 					binPath:          os.Getenv("CHROMIUM_BIN_PATH"),
@@ -503,7 +503,7 @@ func TestChromiumBrowser_pdf(t *testing.T) {
 			noDeadline:    false,
 			start:         true,
 			expectError:   true,
-			expectedError: ErrConnectionRefused,
+			expectedError: ErrLoadingFailed,
 		},
 		{
 			scenario: "clear cache",
@@ -1553,7 +1553,7 @@ func TestChromiumBrowser_screenshot(t *testing.T) {
 			expectedError: ErrConsoleExceptions,
 		},
 		{
-			scenario: "ErrConnectionRefused",
+			scenario: "ErrLoadingFailed",
 			browser: newChromiumBrowser(
 				browserArguments{
 					binPath:          os.Getenv("CHROMIUM_BIN_PATH"),
@@ -1577,7 +1577,7 @@ func TestChromiumBrowser_screenshot(t *testing.T) {
 			noDeadline:    false,
 			start:         true,
 			expectError:   true,
-			expectedError: ErrConnectionRefused,
+			expectedError: ErrLoadingFailed,
 		},
 		{
 			scenario: "clear cache",
--- a/pkg/modules/chromium/chromium.go
+++ b/pkg/modules/chromium/chromium.go
@@ -42,8 +42,8 @@ var (
 	// is set to true.
 	ErrConsoleExceptions = errors.New("console exceptions")

-	// ErrConnectionRefused happens when a URL cannot be reached.
-	ErrConnectionRefused = errors.New("connection refused")
+	// ErrLoadingFailed happens when a URL failed to load.
+	ErrLoadingFailed = errors.New("loading failed")

 	// PDF specific.

--- a/pkg/modules/chromium/events.go
+++ b/pkg/modules/chromium/events.go
@@ -21,7 +21,8 @@ import (
 )

 // listenForEventRequestPaused listens for requests to check if they are
-// allowed or not.
+// allowed or not.network.SetBlockedURLS()
+// TODO: https://chromedevtools.github.io/devtools-protocol/tot/Network/#method-setBlockedURLs (experimental for now).
 func listenForEventRequestPaused(ctx context.Context, logger *zap.Logger, allowList *regexp2.Regexp, denyList *regexp2.Regexp) {
 	chromedp.ListenTarget(ctx, func(ev interface{}) {
 		switch e := ev.(type) {
@@ -95,24 +96,48 @@ func listenForEventResponseReceived(ctx context.Context, logger *zap.Logger, url
 	})
 }

-// listenForEventLoadingFailedOnConnectionRefused listens for an event
-// indicating that the main page failed to load.
-// See https://github.com/gotenberg/gotenberg/issues/913.
-func listenForEventLoadingFailedOnConnectionRefused(ctx context.Context, logger *zap.Logger, connectionRefused *error, connectionRefusedMu *sync.RWMutex) {
+// listenForEventLoadingFailed listens for an event indicating that the main
+// page failed to load.
+// See:
+// https://github.com/gotenberg/gotenberg/issues/913.
+// https://github.com/gotenberg/gotenberg/issues/959.
+func listenForEventLoadingFailed(ctx context.Context, logger *zap.Logger, loadingFailed *error, loadingFailedMu *sync.RWMutex) {
 	chromedp.ListenTarget(ctx, func(ev interface{}) {
 		switch ev := ev.(type) {
 		case *network.EventLoadingFailed:
 			logger.Debug(fmt.Sprintf("event EventLoadingFailed fired: %+v", ev.ErrorText))

-			if ev.ErrorText != "net::ERR_CONNECTION_REFUSED" || ev.Type != network.ResourceTypeDocument {
-				logger.Debug("skip EventLoadingFailed: is not net::ERR_CONNECTION_REFUSED and/or resource type Document")
+			if ev.Type != network.ResourceTypeDocument {
+				logger.Debug("skip EventLoadingFailed: is not resource type Document")
 				return
 			}

-			connectionRefusedMu.Lock()
-			defer connectionRefusedMu.Unlock()
+			// Supposition: except iframe, an event loading failed with a
+			// resource type Document is about the main page.

-			*connectionRefused = fmt.Errorf("%s", ev.ErrorText)
+			// We are looking for common errors.
+			// TODO: sufficient?
+			errors := []string{
+				"net::ERR_CONNECTION_CLOSED",
+				"net::ERR_CONNECTION_RESET",
+				"net::ERR_CONNECTION_REFUSED",
+				"net::ERR_CONNECTION_ABORTED",
+				"net::ERR_CONNECTION_FAILED",
+				"net::ERR_NAME_NOT_RESOLVED",
+				"net::ERR_INTERNET_DISCONNECTED",
+				"net::ERR_ADDRESS_UNREACHABLE",
+				"net::ERR_BLOCKED_BY_CLIENT",
+				"net::ERR_BLOCKED_BY_RESPONSE",
+			}
+			if !slices.Contains(errors, ev.ErrorText) {
+				logger.Debug(fmt.Sprintf("skip EventLoadingFailed: '%s' is not part of %+v", ev.ErrorText, errors))
+				return
+			}
+
+			loadingFailedMu.Lock()
+			defer loadingFailedMu.Unlock()
+
+			*loadingFailed = fmt.Errorf("%s", ev.ErrorText)
 		}
 	})
 }
--- a/pkg/modules/chromium/routes.go
+++ b/pkg/modules/chromium/routes.go
@@ -688,12 +688,12 @@ func handleChromiumError(err error, options Options) error {
 		)
 	}

-	if errors.Is(err, ErrConnectionRefused) {
+	if errors.Is(err, ErrLoadingFailed) {
 		return api.WrapError(
 			err,
 			api.NewSentinelHttpError(
 				http.StatusBadRequest,
-				"Chromium returned net::ERR_CONNECTION_REFUSED",
+				fmt.Sprintf("Chromium returned %v", err),
 			),
 		)
 	}
--- a/pkg/modules/chromium/routes_test.go
+++ b/pkg/modules/chromium/routes_test.go
@@ -1436,10 +1436,10 @@ func TestConvertUrl(t *testing.T) {
 			expectOutputPathsCount: 0,
 		},
 		{
-			scenario: "ErrConnectionRefused",
+			scenario: "ErrLoadingFailed",
 			ctx:      &api.ContextMock{Context: new(api.Context)},
 			api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error {
-				return ErrConnectionRefused
+				return ErrLoadingFailed
 			}},
 			options:                DefaultPdfOptions(),
 			expectError:            true,
@@ -1646,10 +1646,10 @@ func TestScreenshotUrl(t *testing.T) {
 			expectOutputPathsCount: 0,
 		},
 		{
-			scenario: "ErrConnectionRefused",
+			scenario: "ErrLoadingFailed",
 			ctx:      &api.ContextMock{Context: new(api.Context)},
 			api: &ApiMock{ScreenshotMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options ScreenshotOptions) error {
-				return ErrConnectionRefused
+				return ErrLoadingFailed
 			}},
 			options:                DefaultScreenshotOptions(),
 			expectError:            true,