Repository: gotenberg/gotenberg (go)

Issue:
URL convert render empty PDF on error
Hello, thanks for this project :) 


it seems that for some reason if you try to render an URL, but the site is not reachable (ie not route to host)

then it render an empty pdf instead of an error

repro :
```
curl --request POST \
  --url http://localhost:9995/forms/chromium/convert/url \
  --header 'Content-Type: multipart/form-data' \
  --form url=http://19226565944.com
```
will give you an empty pdf 


version 8.9.1

Pull request:
fix(chromium): increase the scope of loading failed errors
Fixes #959.

Patch:
diff --git a/pkg/modules/chromium/browser.go b/pkg/modules/chromium/browser.go
index 471efcd..e38c83b 100644
--- 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
diff --git a/pkg/modules/chromium/chromium.go b/pkg/modules/chromium/chromium.go
index 8e3806e..e5f8f40 100644
--- 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.
 
diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go
index f43b2c8..8a8ca1d 100644
--- 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)
 		}
 	})
 }
diff --git a/pkg/modules/chromium/routes.go b/pkg/modules/chromium/routes.go
index 6edee14..ba1e7be 100644
--- 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),
 			),
 		)
 	}


Test patch:
diff --git a/pkg/modules/chromium/browser_test.go b/pkg/modules/chromium/browser_test.go
index 76764b7..d1c54a4 100644
--- 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",
diff --git a/pkg/modules/chromium/routes_test.go b/pkg/modules/chromium/routes_test.go
index e09c161..ba957e8 100644
--- 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,