fix(chromium): register lifecycle listeners before navigate to close race

pkg/modules/chromium/events.go

@@ -428,19 +428,24 @@ func listenForEventExceptionThrown(ctx context.Context, logger *slog.Logger, con
 	})
 }
 
-// waitForEventDomContentEventFired waits until the event DomContentEventFired
+// waitForEventDomContentEventFired registers a listener for the
-// is fired or the context timeout.
+// DomContentEventFired event and returns a waiter that blocks until the
+// event fires or ctx is done. The listener registers at call time, not
+// inside the waiter, so callers must invoke this before triggering the
+// action that may emit the event. Registering inside the waiter would
+// open a race: for fast loads (typically file:// pages with no external
+// sub-resources), the event can fire before the waiter goroutine starts
+// and the listener never sees a record.
 func waitForEventDomContentEventFired(ctx context.Context, logger *slog.Logger) func() error {
+	ch := make(chan struct{})
+	cctx, cancel := context.WithCancel(ctx)
+	chromedp.ListenTarget(cctx, func(ev any) {
+		if _, ok := ev.(*page.EventDomContentEventFired); ok {
+			cancel()
+			close(ch)
+		}
+	})
 	return func() error {
-		ch := make(chan struct{})
-		cctx, cancel := context.WithCancel(ctx)
-		chromedp.ListenTarget(cctx, func(ev any) {
-			if _, ok := ev.(*page.EventDomContentEventFired); ok {
-				cancel()
-				close(ch)
-			}
-		})
-
 		select {
 		case <-ch:
 			logger.DebugContext(ctx, "event DomContentEventFired fired")
@@ -451,19 +456,20 @@ func waitForEventDomContentEventFired(ctx context.Context, logger *slog.Logger)
 	}
 }
 
-// waitForEventLoadEventFired waits until the event LoadEventFired is fired or
+// waitForEventLoadEventFired registers a listener for the LoadEventFired
-// the context timeout.
+// event and returns a waiter that blocks until the event fires or ctx is
+// done. See [waitForEventDomContentEventFired] for the rationale on
+// registering at call time rather than inside the waiter.
 func waitForEventLoadEventFired(ctx context.Context, logger *slog.Logger) func() error {
+	ch := make(chan struct{})
+	cctx, cancel := context.WithCancel(ctx)
+	chromedp.ListenTarget(cctx, func(ev any) {
+		if _, ok := ev.(*page.EventLoadEventFired); ok {
+			cancel()
+			close(ch)
+		}
+	})
 	return func() error {
-		ch := make(chan struct{})
-		cctx, cancel := context.WithCancel(ctx)
-		chromedp.ListenTarget(cctx, func(ev any) {
-			if _, ok := ev.(*page.EventLoadEventFired); ok {
-				cancel()
-				close(ch)
-			}
-		})
-
 		select {
 		case <-ch:
 			logger.DebugContext(ctx, "event LoadEventFired fired")
@@ -474,19 +480,20 @@ func waitForEventLoadEventFired(ctx context.Context, logger *slog.Logger) func()
 	}
 }
 
-// waitForEventNetworkIdle waits until the event networkIdle is fired or the
+// waitForEventNetworkIdle registers a listener for the networkIdle
-// context timeout.
+// lifecycle event and returns a waiter that blocks until the event fires
+// or ctx is done. See [waitForEventDomContentEventFired] for the
+// rationale on registering at call time rather than inside the waiter.
 func waitForEventNetworkIdle(ctx context.Context, logger *slog.Logger) func() error {
+	ch := make(chan struct{})
+	cctx, cancel := context.WithCancel(ctx)
+	chromedp.ListenTarget(cctx, func(ev any) {
+		if e, ok := ev.(*page.EventLifecycleEvent); ok && e.Name == "networkIdle" {
+			cancel()
+			close(ch)
+		}
+	})
 	return func() error {
-		ch := make(chan struct{})
-		cctx, cancel := context.WithCancel(ctx)
-		chromedp.ListenTarget(cctx, func(ev any) {
-			if e, ok := ev.(*page.EventLifecycleEvent); ok && e.Name == "networkIdle" {
-				cancel()
-				close(ch)
-			}
-		})
-
 		select {
 		case <-ch:
 			logger.DebugContext(ctx, "event networkIdle fired")
@@ -497,19 +504,20 @@ func waitForEventNetworkIdle(ctx context.Context, logger *slog.Logger) func() er
 	}
 }
 
-// waitForEventNetworkAlmostIdle waits until the event networkIdle2 is fired
+// waitForEventNetworkAlmostIdle registers a listener for the networkIdle2
-// or the context timeout.
+// lifecycle event and returns a waiter that blocks until the event fires
+// or ctx is done. See [waitForEventDomContentEventFired] for the
+// rationale on registering at call time rather than inside the waiter.
 func waitForEventNetworkAlmostIdle(ctx context.Context, logger *slog.Logger) func() error {
+	ch := make(chan struct{})
+	cctx, cancel := context.WithCancel(ctx)
+	chromedp.ListenTarget(cctx, func(ev any) {
+		if e, ok := ev.(*page.EventLifecycleEvent); ok && e.Name == "networkIdle2" {
+			cancel()
+			close(ch)
+		}
+	})
 	return func() error {
-		ch := make(chan struct{})
-		cctx, cancel := context.WithCancel(ctx)
-		chromedp.ListenTarget(cctx, func(ev any) {
-			if e, ok := ev.(*page.EventLifecycleEvent); ok && e.Name == "networkIdle2" {
-				cancel()
-				close(ch)
-			}
-		})
-
 		select {
 		case <-ch:
 			logger.DebugContext(ctx, "event networkAlmostIdle fired")
@@ -520,19 +528,20 @@ func waitForEventNetworkAlmostIdle(ctx context.Context, logger *slog.Logger) fun
 	}
 }
 
-// waitForEventLoadingFinished waits until the event LoadingFinished is fired
+// waitForEventLoadingFinished registers a listener for the
-// or the context timeout.
+// LoadingFinished event and returns a waiter that blocks until the event
+// fires or ctx is done. See [waitForEventDomContentEventFired] for the
+// rationale on registering at call time rather than inside the waiter.
 func waitForEventLoadingFinished(ctx context.Context, logger *slog.Logger) func() error {
+	ch := make(chan struct{})
+	cctx, cancel := context.WithCancel(ctx)
+	chromedp.ListenTarget(cctx, func(ev any) {
+		if _, ok := ev.(*network.EventLoadingFinished); ok {
+			cancel()
+			close(ch)
+		}
+	})
 	return func() error {
-		ch := make(chan struct{})
-		cctx, cancel := context.WithCancel(ctx)
-		chromedp.ListenTarget(cctx, func(ev any) {
-			if _, ok := ev.(*network.EventLoadingFinished); ok {
-				cancel()
-				close(ch)
-			}
-		})
-
 		select {
 		case <-ch:
 			logger.DebugContext(ctx, "event LoadingFinished fired")

pkg/modules/chromium/tasks.go

@@ -332,11 +332,13 @@ func navigateActionFunc(logger *slog.Logger, url string, skipNetworkIdleEvent, s
 	return func(ctx context.Context) error {
 		logger.DebugContext(ctx, fmt.Sprintf("navigate to '%s'", url))
 
-		_, _, _, _, err := page.Navigate(url).Do(ctx)
+		// Register lifecycle listeners before issuing Page.navigate. For
-		if err != nil {
+		// fast loads (typically file:// pages with no external
-			return fmt.Errorf("navigate to '%s': %w", url, err)
+		// sub-resources), DomContentEventFired / LoadEventFired /
-		}
+		// LoadingFinished can fire between Navigate.Do returning and
-
+		// runBatch spawning the waiter goroutines. Registering ahead of
+		// the navigate command closes that race.
+		// See https://github.com/gotenberg/gotenberg/issues/1561.
 		waitFunc := []func() error{
 			waitForEventDomContentEventFired(ctx, logger),
 			waitForEventLoadEventFired(ctx, logger),
@@ -355,6 +357,11 @@ func navigateActionFunc(logger *slog.Logger, url string, skipNetworkIdleEvent, s
 			logger.DebugContext(ctx, "skipping network almost idle event")
 		}
 
+		_, _, _, _, err := page.Navigate(url).Do(ctx)
+		if err != nil {
+			return fmt.Errorf("navigate to '%s': %w", url, err)
+		}
+
 		err = runBatch(
 			ctx,
 			waitFunc...,