From b73207f29abf2b573d21edcdb32e82f15070dfbe Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Sun, 16 Oct 2022 12:34:22 +0200 Subject: [PATCH] fix: websocket timeout reached (fixes #524) --- pkg/modules/chromium/chromium.go | 8 +++ pkg/modules/chromium/chromium_test.go | 98 +++++++++++++++++++++------ 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/pkg/modules/chromium/chromium.go b/pkg/modules/chromium/chromium.go index fde7193b..a5bbb646 100644 --- a/pkg/modules/chromium/chromium.go +++ b/pkg/modules/chromium/chromium.go @@ -420,6 +420,14 @@ func (mod Chromium) PDF(ctx context.Context, logger *zap.Logger, URL, outputPath args = append(args, chromedp.UserAgent(options.UserAgent)) } + // See https://github.com/gotenberg/gotenberg/issues/524. + deadline, ok := ctx.Deadline() + if !ok { + return errors.New("context has no deadline") + } + + args = append(args, chromedp.WSURLReadTimeout(time.Until(deadline))) + allocatorCtx, cancel := chromedp.NewExecAllocator(ctx, args...) defer cancel() diff --git a/pkg/modules/chromium/chromium_test.go b/pkg/modules/chromium/chromium_test.go index 1f11050b..3aeefa48 100644 --- a/pkg/modules/chromium/chromium_test.go +++ b/pkg/modules/chromium/chromium_test.go @@ -224,7 +224,8 @@ func TestChromium_Routes(t *testing.T) { } func TestChromium_PDF(t *testing.T) { - for i, tc := range []struct { + for _, tc := range []struct { + name string timeout time.Duration cancel context.CancelFunc URL string @@ -243,34 +244,51 @@ func TestChromium_PDF(t *testing.T) { expectErr bool }{ { + name: "context has no deadline", + URL: "file:///tests/test/testdata/chromium/html/sample1/index.html", + expectErr: true, + }, + { + name: "URL does not match the expression from the allowed list", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", allowList: regexp.MustCompile("file:///tmp/*"), expectErr: true, }, { + name: "URL does not match the expression from the denied list", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", denyList: regexp.MustCompile("file:///tests/*"), expectErr: true, }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with user agent", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ UserAgent: "foo", }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample10/index.html", + name: "fail on console exceptions", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample10/index.html", options: Options{ FailOnConsoleExceptions: true, }, expectErr: true, }, { + name: "disable JavaScript", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample9/index.html", disableJavaScript: true, }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with extra HTTP headers", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ ExtraHTTPHeaders: map[string]string{ "foo": "bar", @@ -278,7 +296,9 @@ func TestChromium_PDF(t *testing.T) { }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample11/index.html", + name: "with extra link tags", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample11/index.html", options: Options{ ExtraLinkTags: []LinkTag{ { @@ -291,26 +311,34 @@ func TestChromium_PDF(t *testing.T) { }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample8/index.html", + name: "with invalid emulated media type", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample8/index.html", options: Options{ EmulatedMediaType: "foo", }, expectErr: true, }, { - URL: "file:///tests/test/testdata/chromium/html/sample8/index.html", + name: "with screen emulated media type", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample8/index.html", options: Options{ EmulatedMediaType: "screen", }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample8/index.html", + name: "with print emulated media type", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample8/index.html", options: Options{ EmulatedMediaType: "print", }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample11/index.html", + name: "with extra script tags", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample11/index.html", options: Options{ ExtraScriptTags: []ScriptTag{ { @@ -320,12 +348,15 @@ func TestChromium_PDF(t *testing.T) { }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with wait delay", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ WaitDelay: time.Duration(1) * time.Nanosecond, }, }, { + name: "with invalid wait window status", timeout: time.Duration(3) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample2/index.html", options: Options{ @@ -334,6 +365,7 @@ func TestChromium_PDF(t *testing.T) { expectErr: true, }, { + name: "with wait window status", timeout: time.Duration(3) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample2/index.html", options: Options{ @@ -341,6 +373,7 @@ func TestChromium_PDF(t *testing.T) { }, }, { + name: "with wait for expression that should not happen", timeout: time.Duration(3) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample2/index.html", options: Options{ @@ -349,6 +382,7 @@ func TestChromium_PDF(t *testing.T) { expectErr: true, }, { + name: "with valid wait for expression", timeout: time.Duration(3) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample2/index.html", options: Options{ @@ -356,27 +390,35 @@ func TestChromium_PDF(t *testing.T) { }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with invalid wait for expression", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ WaitForExpression: "return undefined", }, expectErr: true, }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with too big margin bottom", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ MarginBottom: 100, }, expectErr: true, }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with invalid page ranges", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ PageRanges: "foo", }, expectErr: true, }, { + name: "with a lot of options", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", userAgent: "foo", incognito: true, @@ -388,18 +430,26 @@ func TestChromium_PDF(t *testing.T) { proxyServer: "foo", }, { - URL: "file:///tests/test/testdata/chromium/html/sample1/index.html", + name: "with file using local and remote assets", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample1/index.html", }, { + name: "URL does match the expression from the allowed list", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample3/index.html", allowList: regexp.MustCompile("file:///tests/*"), }, { + name: "URL does match the expression from the denied list", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample3/index.html", denyList: regexp.MustCompile("file:///etc/*"), }, { - URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", + name: "with custom header and footer templates", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample4/index.html", options: Options{ HeaderTemplate: func() string { b, err := os.ReadFile("/tests/test/testdata/chromium/url/sample2/header.html") @@ -420,14 +470,20 @@ func TestChromium_PDF(t *testing.T) { }, }, { - URL: "file:///tests/test/testdata/chromium/html/sample5/index.html", + name: "with file using a .gif", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample5/index.html", }, { + name: "with allow file access from files", + timeout: time.Duration(60) * time.Second, URL: "file:///tests/test/testdata/chromium/html/sample6/index.html", allowFileAccessFromFiles: true, }, { - URL: "file:///tests/test/testdata/chromium/html/sample7/index.html", + name: "with file using a style attribute", + timeout: time.Duration(60) * time.Second, + URL: "file:///tests/test/testdata/chromium/html/sample7/index.html", }, } { func() { @@ -456,13 +512,13 @@ func TestChromium_PDF(t *testing.T) { outputDir, err := gotenberg.MkdirAll() if err != nil { - t.Fatalf("test %d: expected error but got: %v", i, err) + t.Fatalf("test %s: expected error but got: %v", tc.name, err) } defer func() { err := os.RemoveAll(outputDir) if err != nil { - t.Fatalf("test %d: expected no error but got: %v", i, err) + t.Fatalf("test %s: expected no error but got: %v", tc.name, err) } }() @@ -476,11 +532,11 @@ func TestChromium_PDF(t *testing.T) { } if tc.expectErr && err == nil { - t.Errorf("test %d: expected error but got: %v", i, err) + t.Errorf("test %s: expected error but got: %v", tc.name, err) } if !tc.expectErr && err != nil { - t.Errorf("test %d: expected no error but got: %v", i, err) + t.Errorf("test %s: expected no error but got: %v", tc.name, err) } }() }