From 9af00d6d583ebfa549051fdbc867d34a4ce32453 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Mon, 8 Jul 2019 17:24:17 +0200 Subject: [PATCH] fixes waitTimeout, waitDelay not working with float --- .../docs/content/03-environment-variables.md | 5 ++- build/docs/content/04-html.md | 5 ++- docs/index.html | 5 ++- internal/app/api/pkg/context/context.go | 1 - internal/pkg/printer/chrome.go | 8 ++-- internal/pkg/printer/merge.go | 5 +-- internal/pkg/printer/office.go | 5 +-- internal/pkg/timeout/doc.go | 3 ++ internal/pkg/timeout/timeout.go | 43 +++++++++++++++++++ internal/pkg/timeout/timeout_test.go | 38 ++++++++++++++++ test/testfunc.go | 10 +++++ 11 files changed, 110 insertions(+), 18 deletions(-) create mode 100644 internal/pkg/timeout/doc.go create mode 100644 internal/pkg/timeout/timeout.go create mode 100644 internal/pkg/timeout/timeout_test.go diff --git a/build/docs/content/03-environment-variables.md b/build/docs/content/03-environment-variables.md index 70653291..a9f75d32 100644 --- a/build/docs/content/03-environment-variables.md +++ b/build/docs/content/03-environment-variables.md @@ -37,7 +37,8 @@ By default, the API will add a log entry when the [healthcheck endpoint](#ping) You may turn off this logging so as to avoid unnecessary entries in your logs with the environment variable `DISABLE_HEALTHCHECK_LOGGING`. -This environment variable operates in the same manner as the `DISABLE_GOOGLE_CHROME` and `DISABLE_UNOCONV` variables operate in that it accepts the strings `"0"` or `"1"` as values, where `1` is enabled. +This environment variable operates in the same manner as the `DISABLE_GOOGLE_CHROME` and `DISABLE_UNOCONV` variables operate in that it accepts the strings `"0"` or `"1"` as values, where `"1"` is enabled. + ## Default listen port By default, the API will listen on port `3000`. For most use cases this is perfectly fine, but at times there may be cases where you need to change this due to port conflicts. @@ -52,4 +53,4 @@ By default, `stdout` and `stderr` messages from the started processes are disabl You may enable some debug logging from starting the process by setting the environment variable `DEBUG_PROCESS_STARTUP`. -This environment variable operates in the same manner as the `DISABLE_GOOGLE_CHROME` and `DISABLE_UNOCONV` variables operate in that it accepts the strings `"0"` or `"1"` as values, where `1` means `true`. \ No newline at end of file +This environment variable operates in the same manner as the `DISABLE_GOOGLE_CHROME` and `DISABLE_UNOCONV` variables operate in that it accepts the strings `"0"` or `"1"` as values, where `1` means `true`. diff --git a/build/docs/content/04-html.md b/build/docs/content/04-html.md index b2661e3c..4fdb6741 100644 --- a/build/docs/content/04-html.md +++ b/build/docs/content/04-html.md @@ -296,7 +296,8 @@ $client->store($request, $dest); ## Wait delay In some cases, you may want to wait a certain amount of time to make sure the -page you're trying to generate is fully rendered. +page you're trying to generate is fully rendered. For instance, if your page relies +a lot on JavaScript for rendering. > The wait delay is a duration in **seconds** (e.g `2.5` for 2.5 seconds). @@ -339,4 +340,4 @@ $request = new HTMLRequest($index); $request->setWaitDelay(5.5); $dest = "result.pdf"; $client->store($request, $dest); -``` \ No newline at end of file +``` diff --git a/docs/index.html b/docs/index.html index 6e1a1677..47e58226 100755 --- a/docs/index.html +++ b/docs/index.html @@ -272,7 +272,7 @@ See the timeout section.

You may turn off this logging so as to avoid unnecessary entries in your logs with the environment variable DISABLE_HEALTHCHECK_LOGGING.

-

This environment variable operates in the same manner as the DISABLE_GOOGLE_CHROME and DISABLE_UNOCONV variables operate in that it accepts the strings "0" or "1" as values, where 1 is enabled.

+

This environment variable operates in the same manner as the DISABLE_GOOGLE_CHROME and DISABLE_UNOCONV variables operate in that it accepts the strings "0" or "1" as values, where "1" is enabled.

Wait delay

In some cases, you may want to wait a certain amount of time to make sure the -page you’re trying to generate is fully rendered.

+page you’re trying to generate is fully rendered. For instance, if your page relies +a lot on JavaScript for rendering.

The wait delay is a duration in seconds (e.g 2.5 for 2.5 seconds).

diff --git a/internal/app/api/pkg/context/context.go b/internal/app/api/pkg/context/context.go index 1ee8ab5b..e3673c35 100644 --- a/internal/app/api/pkg/context/context.go +++ b/internal/app/api/pkg/context/context.go @@ -23,7 +23,6 @@ type Context struct { // New creates a new context. func New(c echo.Context, logger *logger.Logger, config *config.Config) *Context { - // TODO timeout context? return &Context{ c, logger, diff --git a/internal/pkg/printer/chrome.go b/internal/pkg/printer/chrome.go index 3e1a483d..5bf36a9c 100644 --- a/internal/pkg/printer/chrome.go +++ b/internal/pkg/printer/chrome.go @@ -13,6 +13,7 @@ import ( "github.com/mafredri/cdp/protocol/target" "github.com/mafredri/cdp/rpcc" "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" + "github.com/thecodingmachine/gotenberg/internal/pkg/timeout" "golang.org/x/sync/errgroup" ) @@ -39,9 +40,7 @@ type ChromeOptions struct { func (p *chrome) Print(destination string) error { const op = "printer.chrome.Print" - // FIXME duration not working with float - duration := time.Duration(p.opts.WaitTimeout+p.opts.WaitDelay) * time.Second - ctx, cancel := context.WithTimeout(context.Background(), duration) + ctx, cancel := timeout.Context(p.opts.WaitTimeout + p.opts.WaitDelay) defer cancel() devt, err := devtool.New("http://localhost:9222").Version(ctx) if err != nil { @@ -153,8 +152,7 @@ func (p *chrome) navigate(ctx context.Context, client *cdp.Client) error { return &standarderror.Error{Op: op, Err: err} } // wait for a given amount of time (useful for javascript delay). - // FIXME duration not working with float - time.Sleep(time.Duration(p.opts.WaitDelay) * time.Second) + time.Sleep(timeout.Duration(p.opts.WaitDelay)) return nil } diff --git a/internal/pkg/printer/merge.go b/internal/pkg/printer/merge.go index 6393f5cf..cc096801 100644 --- a/internal/pkg/printer/merge.go +++ b/internal/pkg/printer/merge.go @@ -3,9 +3,9 @@ package printer import ( "context" "os/exec" - "time" "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" + "github.com/thecodingmachine/gotenberg/internal/pkg/timeout" ) type merge struct { @@ -31,8 +31,7 @@ func NewMerge(fpaths []string, opts *MergeOptions) Printer { func (p *merge) Print(destination string) error { const op = "printer.merge.Print" if p.ctx == nil { - // FIXME duration not working with float - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(p.opts.WaitTimeout)*time.Second) + ctx, cancel := timeout.Context(p.opts.WaitTimeout) defer cancel() p.ctx = ctx } diff --git a/internal/pkg/printer/office.go b/internal/pkg/printer/office.go index 214da42b..d3eaec61 100644 --- a/internal/pkg/printer/office.go +++ b/internal/pkg/printer/office.go @@ -7,10 +7,10 @@ import ( "os/exec" "path/filepath" "sync" - "time" "github.com/labstack/gommon/random" "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" + "github.com/thecodingmachine/gotenberg/internal/pkg/timeout" ) type office struct { @@ -35,8 +35,7 @@ func NewOffice(fpaths []string, opts *OfficeOptions) Printer { func (p *office) Print(destination string) error { const op = "printer.office.Print" - // FIXME duration not working with float - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(p.opts.WaitTimeout)*time.Second) + ctx, cancel := timeout.Context(p.opts.WaitTimeout) defer cancel() fpaths := make([]string, len(p.fpaths)) dirPath := filepath.Dir(destination) diff --git a/internal/pkg/timeout/doc.go b/internal/pkg/timeout/doc.go new file mode 100644 index 00000000..e8c95004 --- /dev/null +++ b/internal/pkg/timeout/doc.go @@ -0,0 +1,3 @@ +// Package timeout helps managing +// context with timeout. +package timeout diff --git a/internal/pkg/timeout/timeout.go b/internal/pkg/timeout/timeout.go new file mode 100644 index 00000000..7228ece2 --- /dev/null +++ b/internal/pkg/timeout/timeout.go @@ -0,0 +1,43 @@ +package timeout + +import ( + "context" + "strings" + "time" + + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" +) + +// Context creates a context with timeout for +// given second. +func Context(seconds float64) (context.Context, context.CancelFunc) { + return context.WithTimeout(context.Background(), Duration(seconds)) +} + +// Duration creates a duration from seconds. +func Duration(seconds float64) time.Duration { + return time.Duration(1000*seconds) * time.Millisecond +} + +// Err returns a standarderror.Error +// if the context has an error. +func Err(ctx context.Context) error { + const op = "timeout.Err" + err := ctx.Err() + if err == nil { + return nil + } + if strings.Contains(err.Error(), context.DeadlineExceeded.Error()) { + return &standarderror.Error{ + Code: standarderror.Timeout, + Message: "context has timed out", + Op: op, + Err: err, + } + } + return &standarderror.Error{ + Message: "context finished with an error", + Op: op, + Err: err, + } +} diff --git a/internal/pkg/timeout/timeout_test.go b/internal/pkg/timeout/timeout_test.go new file mode 100644 index 00000000..23d4c06a --- /dev/null +++ b/internal/pkg/timeout/timeout_test.go @@ -0,0 +1,38 @@ +package timeout + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" + "github.com/thecodingmachine/gotenberg/test" +) + +func TestDuration(t *testing.T) { + expected := time.Duration(1500) * time.Millisecond + result := Duration(1.5) + assert.Equal(t, expected.String(), result.String()) +} + +func TestErr(t *testing.T) { + // should be OK. + ctx, cancel := Context(5) + defer cancel() + assert.Nil(t, Err(ctx)) + // should timeout. + ctx, cancel = Context(0.5) + defer cancel() + time.Sleep(Duration(1)) + err := Err(ctx) + assert.NotNil(t, err) + standardized := test.RequireStandardError(t, err) + assert.Equal(t, standardized.Code, standarderror.Timeout) + // should failed. + ctx, cancel = Context(5) + cancel() + err = Err(ctx) + assert.NotNil(t, err) + standardized = test.RequireStandardError(t, err) + assert.Equal(t, standarderror.Code(err), standarderror.Internal) +} diff --git a/test/testfunc.go b/test/testfunc.go index acdbafa0..bf1103b2 100644 --- a/test/testfunc.go +++ b/test/testfunc.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" "golang.org/x/sync/errgroup" ) @@ -39,6 +40,15 @@ func AssertConcurrent(t *testing.T, fn func() error, amount int) { assert.NoError(t, err) } +// RequireStandardError validates that given error +// is of an instance of standarderror.Error. +// If so, returns the instance of standarderror.Error. +func RequireStandardError(t *testing.T, err error) *standarderror.Error { + standardized, ok := err.(*standarderror.Error) + require.Equal(t, ok, true) + return standardized +} + // HTMLTestMultipartForm returns the body // for a multipate/form-data request with all // files under "html" folder.