From 5af01505c2148fa15997742416b45ae9132aa54d Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Mon, 8 Jul 2019 10:28:43 +0200 Subject: [PATCH] better op names + fixing issue with op chaining in errors --- internal/app/api/pkg/handler/handler.go | 25 +++++++-------------- internal/app/api/pkg/handler/html.go | 11 ++++++--- internal/app/api/pkg/handler/markdown.go | 13 +++++++---- internal/app/api/pkg/handler/merge.go | 11 ++++++--- internal/app/api/pkg/handler/office.go | 11 ++++++--- internal/app/api/pkg/handler/url.go | 11 ++++++--- internal/app/api/pkg/resource/resource.go | 21 ++++++++--------- internal/pkg/config/config.go | 8 +++---- internal/pkg/pm2/chrome.go | 14 ++++++------ internal/pkg/pm2/unoconv.go | 4 ++-- internal/pkg/standarderror/standarderror.go | 9 +++----- 11 files changed, 74 insertions(+), 64 deletions(-) diff --git a/internal/app/api/pkg/handler/handler.go b/internal/app/api/pkg/handler/handler.go index c15b29ea..b511818c 100644 --- a/internal/app/api/pkg/handler/handler.go +++ b/internal/app/api/pkg/handler/handler.go @@ -35,10 +35,7 @@ const ( ) func convert(ctx *context.Context, p printer.Printer) error { - const ( - op = "convert" - debugOp = "handler.convert" - ) + const op = "handler.convert" r := ctx.Resource() logger := ctx.StandardLogger() baseFilename := random.Get() @@ -48,7 +45,7 @@ func convert(ctx *context.Context, p printer.Printer) error { // and directly return the resulting PDF file // or an error. if !r.Has(resource.WebhookURLFormField) { - logger.DebugfOp(debugOp, "no '%s' found, converting synchronously", resource.WebhookURLFormField) + logger.DebugfOp(op, "no '%s' found, converting synchronously", resource.WebhookURLFormField) if err := convertSync(filename, fpath, ctx, p); err != nil { return &standarderror.Error{Op: op, Err: err} } @@ -57,15 +54,12 @@ func convert(ctx *context.Context, p printer.Printer) error { // as a webhook URL has been given, we // run the following lines in a goroutine so that // it doesn't block. - logger.DebugfOp(debugOp, "'%s' found, converting asynchronously", resource.WebhookURLFormField) + logger.DebugfOp(op, "'%s' found, converting asynchronously", resource.WebhookURLFormField) return convertAsync(filename, fpath, ctx, p) } func convertSync(filename, fpath string, ctx *context.Context, p printer.Printer) error { - const ( - op = "convertSync" - debugOp = "handler.convertSync" - ) + const op = "handler.convertSync" r := ctx.Resource() logger := ctx.StandardLogger() if err := p.Print(fpath); err != nil { @@ -73,7 +67,7 @@ func convertSync(filename, fpath string, ctx *context.Context, p printer.Printer } if !r.Has(resource.ResultFilenameFormField) { logger.DebugfOp( - debugOp, + op, "no '%s' found, using generated filename '%s'", resource.ResultFilenameFormField, filename, @@ -84,7 +78,7 @@ func convertSync(filename, fpath string, ctx *context.Context, p printer.Printer return nil } logger.DebugfOp( - debugOp, + op, "'%s' found, so not using generated filename", resource.ResultFilenameFormField, ) @@ -99,10 +93,7 @@ func convertSync(filename, fpath string, ctx *context.Context, p printer.Printer } func convertAsync(filename, fpath string, ctx *context.Context, p printer.Printer) error { - const ( - op = "convertAsync" - debugOp = "handler.convertAsync" - ) + const op = "handler.convertAsync" r := ctx.Resource() logger := ctx.StandardLogger() go func() { @@ -132,7 +123,7 @@ func convertAsync(filename, fpath string, ctx *context.Context, p printer.Printe return } logger.DebugfOp( - debugOp, + op, "sending result file '%s' to '%s'", filename, webhookURL, diff --git a/internal/app/api/pkg/handler/html.go b/internal/app/api/pkg/handler/html.go index d5850685..117e1749 100644 --- a/internal/app/api/pkg/handler/html.go +++ b/internal/app/api/pkg/handler/html.go @@ -4,21 +4,26 @@ import ( "github.com/labstack/echo/v4" "github.com/thecodingmachine/gotenberg/internal/app/api/pkg/context" "github.com/thecodingmachine/gotenberg/internal/pkg/printer" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" ) // HTML is the endpoint for converting // HTML to PDF. func HTML(c echo.Context) error { + const op = "handler.HTML" ctx := context.MustCastFromEchoContext(c) r := ctx.Resource() opts, err := r.ChromePrinterOptions() if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } fpath, err := r.Fpath("index.html") if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } p := printer.NewHTML(fpath, opts) - return convert(ctx, p) + if err := convert(ctx, p); err != nil { + return &standarderror.Error{Op: op, Err: err} + } + return nil } diff --git a/internal/app/api/pkg/handler/markdown.go b/internal/app/api/pkg/handler/markdown.go index a4255fd2..6e22a556 100644 --- a/internal/app/api/pkg/handler/markdown.go +++ b/internal/app/api/pkg/handler/markdown.go @@ -4,24 +4,29 @@ import ( "github.com/labstack/echo/v4" "github.com/thecodingmachine/gotenberg/internal/app/api/pkg/context" "github.com/thecodingmachine/gotenberg/internal/pkg/printer" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" ) // Markdown is the endpoint for converting // Markdown to PDF. func Markdown(c echo.Context) error { + const op = "handler.Markdown" ctx := context.MustCastFromEchoContext(c) r := ctx.Resource() opts, err := r.ChromePrinterOptions() if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } fpath, err := r.Fpath("index.html") if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } p, err := printer.NewMarkdown(fpath, opts) if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } - return convert(ctx, p) + if err := convert(ctx, p); err != nil { + return &standarderror.Error{Op: op, Err: err} + } + return nil } diff --git a/internal/app/api/pkg/handler/merge.go b/internal/app/api/pkg/handler/merge.go index f9d1e5d1..6bd74d27 100644 --- a/internal/app/api/pkg/handler/merge.go +++ b/internal/app/api/pkg/handler/merge.go @@ -4,21 +4,26 @@ import ( "github.com/labstack/echo/v4" "github.com/thecodingmachine/gotenberg/internal/app/api/pkg/context" "github.com/thecodingmachine/gotenberg/internal/pkg/printer" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" ) // Merge is the endpoint for // merging PDF files. func Merge(c echo.Context) error { + const op = "handler.Merge" ctx := context.MustCastFromEchoContext(c) r := ctx.Resource() opts, err := r.MergePrinterOptions() if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } fpaths, err := r.Fpaths(".pdf") if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } p := printer.NewMerge(fpaths, opts) - return convert(ctx, p) + if err := convert(ctx, p); err != nil { + return &standarderror.Error{Op: op, Err: err} + } + return nil } diff --git a/internal/app/api/pkg/handler/office.go b/internal/app/api/pkg/handler/office.go index 3cf5c315..09d294b4 100644 --- a/internal/app/api/pkg/handler/office.go +++ b/internal/app/api/pkg/handler/office.go @@ -4,16 +4,18 @@ import ( "github.com/labstack/echo/v4" "github.com/thecodingmachine/gotenberg/internal/app/api/pkg/context" "github.com/thecodingmachine/gotenberg/internal/pkg/printer" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" ) // Office is the endpoint for converting // Office files to PDF. func Office(c echo.Context) error { + const op = "handler.Office" ctx := context.MustCastFromEchoContext(c) r := ctx.Resource() opts, err := r.OfficePrinterOptions() if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } fpaths, err := r.Fpaths( ".txt", @@ -30,8 +32,11 @@ func Office(c echo.Context) error { ".odp", ) if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } p := printer.NewOffice(fpaths, opts) - return convert(ctx, p) + if err := convert(ctx, p); err != nil { + return &standarderror.Error{Op: op, Err: err} + } + return nil } diff --git a/internal/app/api/pkg/handler/url.go b/internal/app/api/pkg/handler/url.go index cf1bc75d..f38eda0a 100644 --- a/internal/app/api/pkg/handler/url.go +++ b/internal/app/api/pkg/handler/url.go @@ -5,21 +5,26 @@ import ( "github.com/thecodingmachine/gotenberg/internal/app/api/pkg/context" "github.com/thecodingmachine/gotenberg/internal/app/api/pkg/resource" "github.com/thecodingmachine/gotenberg/internal/pkg/printer" + "github.com/thecodingmachine/gotenberg/internal/pkg/standarderror" ) // URL is the endpoint for converting // a URL to PDF. func URL(c echo.Context) error { + const op = "handler.URL" ctx := context.MustCastFromEchoContext(c) r := ctx.Resource() opts, err := r.ChromePrinterOptions() if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } remoteURL, err := r.Get(resource.RemoteURLFormField) if err != nil { - return err + return &standarderror.Error{Op: op, Err: err} } p := printer.NewURL(remoteURL, opts) - return convert(ctx, p) + if err := convert(ctx, p); err != nil { + return &standarderror.Error{Op: op, Err: err} + } + return nil } diff --git a/internal/app/api/pkg/resource/resource.go b/internal/app/api/pkg/resource/resource.go index 67d0eda9..479eb38d 100644 --- a/internal/app/api/pkg/resource/resource.go +++ b/internal/app/api/pkg/resource/resource.go @@ -83,7 +83,7 @@ func New(c echo.Context, logger *logger.Logger, config *config.Config, dirPath s } func formValues(c echo.Context, logger *logger.Logger) map[string]string { - const debugOp = "resource.formValues" + const op = "resource.formValues" v := make(map[string]string) v[ResultFilenameFormField] = c.FormValue(ResultFilenameFormField) v[WaitTimeoutFormField] = c.FormValue(WaitTimeoutFormField) @@ -97,15 +97,12 @@ func formValues(c echo.Context, logger *logger.Logger) map[string]string { v[MarginLeftFormField] = c.FormValue(MarginLeftFormField) v[MarginRightFormField] = c.FormValue(MarginRightFormField) v[LandscapeFormField] = c.FormValue(LandscapeFormField) - logger.DebugfOp(debugOp, "%v", v) + logger.DebugfOp(op, "%v", v) return v } func formFiles(c echo.Context, logger *logger.Logger, dirPath string) error { - const ( - op = "formFiles" - debugOp = "resource.formFiles" - ) + const op = "resource.formFiles" form, err := c.MultipartForm() if err != nil { return &standarderror.Error{Op: op, Err: err} @@ -132,7 +129,7 @@ func formFiles(c echo.Context, logger *logger.Logger, dirPath string) error { if _, err := out.Seek(0, 0); err != nil { return &standarderror.Error{Op: op, Err: err} } - logger.DebugfOp(debugOp, "'%s' created", fh.Filename) + logger.DebugfOp(op, "'%s' created", fh.Filename) } } return nil @@ -293,7 +290,7 @@ func (r *Resource) Get(formField string) (string, error) { } func (r *Resource) value(formField string) (string, error) { - const op = "value" + const op = "resource.value" v, ok := r.formValues[formField] if !ok { return "", &standarderror.Error{ @@ -306,7 +303,7 @@ func (r *Resource) value(formField string) (string, error) { } func (r *Resource) float64(formField string, defaultValue float64) (float64, error) { - const op = "float64" + const op = "resource.float64" if !r.Has(formField) { return defaultValue, nil } @@ -326,7 +323,7 @@ func (r *Resource) float64(formField string, defaultValue float64) (float64, err } func (r *Resource) bool(formField string, defaultValue bool) (bool, error) { - const op = "bool" + const op = "resource.bool" if !r.Has(formField) { return defaultValue, nil } @@ -366,7 +363,7 @@ func (r *Resource) Fpath(filename string) (string, error) { } func (r *Resource) content(filename string, defaultValue string) (string, error) { - const op = "content" + const op = "resource.content" if !r.hasFile(filename) { return defaultValue, nil } @@ -387,7 +384,7 @@ func (r *Resource) Fpaths(exts ...string) ([]string, error) { const op = "resource.Fpaths" var fpaths []string err := filepath.Walk(r.formFilesDirPath, func(path string, info os.FileInfo, _ error) error { - const walkOp = "filepath.Walk" + const walkOp = "resource.filepath.Walk" if info.IsDir() { return nil } diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 10168ff3..59246ede 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -103,7 +103,7 @@ func (c *Config) LogLevel() logrus.Level { } func defaultWaitTimeoutFromEnv(envVar string, defaultValue float64) (float64, error) { - const op = "defaultWaitTimeoutFromEnv" + const op = "config.defaultWaitTimeoutFromEnv" if v, ok := os.LookupEnv(envVar); ok { waitTimeout, err := strconv.ParseFloat(v, 64) if err != nil { @@ -119,7 +119,7 @@ func defaultWaitTimeoutFromEnv(envVar string, defaultValue float64) (float64, er } func defaultListenPortFromEnv(envVar string, defaultValue string) (string, error) { - const op = "defaultListenPortFromEnv" + const op = "config.defaultListenPortFromEnv" if v, ok := os.LookupEnv(envVar); ok { portAsUint, err := strconv.ParseUint(v, 10, 64) if err != nil { @@ -142,7 +142,7 @@ func defaultListenPortFromEnv(envVar string, defaultValue string) (string, error } func boolFromEnv(envVar string, defaultValue bool) (bool, error) { - const op = "boolFromEnv" + const op = "config.boolFromEnv" if v, ok := os.LookupEnv(envVar); ok { if v != "1" && v != "0" { return defaultValue, &standarderror.Error{ @@ -157,7 +157,7 @@ func boolFromEnv(envVar string, defaultValue bool) (bool, error) { } func logLevelFromEnv(envVar string, defaultValue logrus.Level) (logrus.Level, error) { - const op = "logLevelFromEnv" + const op = "config.logLevelFromEnv" if v, ok := os.LookupEnv(envVar); ok { switch v { case "DEBUG": diff --git a/internal/pkg/pm2/chrome.go b/internal/pkg/pm2/chrome.go index 07abc846..120baa50 100644 --- a/internal/pkg/pm2/chrome.go +++ b/internal/pkg/pm2/chrome.go @@ -28,7 +28,7 @@ func (p *chrome) Fullname() string { } func (p *chrome) Start() error { - const op = "chrome.Start" + const op = "pm2.chrome.Start" if err := p.manager.start(p); err != nil { return &standarderror.Error{Op: op, Err: err} } @@ -36,7 +36,7 @@ func (p *chrome) Start() error { } func (p *chrome) Shutdown() error { - const op = "chrome.Shutdown" + const op = "pm2.chrome.Shutdown" if err := p.manager.shutdown(p); err != nil { return &standarderror.Error{Op: op, Err: err} } @@ -67,25 +67,25 @@ func (p *chrome) name() string { } func (p *chrome) viable() bool { - const debugOp = "chrome.viable" + const op = "pm2.chrome.viable" // check if Google Chrome is correctly running. ctx, cancel := context.WithCancel(context.Background()) defer cancel() p.manager.logger.DebugfOp( - debugOp, + op, "checking liveness via debug version endpoint http://localhost:9222/json/version", ) v, err := devtool.New("http://localhost:9222").Version(ctx) if err != nil { p.manager.logger.DebugfOp( - debugOp, + op, "debug version endpoint returned error: %v", err, ) return false } p.manager.logger.DebugfOp( - debugOp, + op, "debug version endpoint returned version info: %+v", *v, ) @@ -93,7 +93,7 @@ func (p *chrome) viable() bool { } func (p *chrome) warmup() { - const debugOp = "chrome.warmup" + const debugOp = "pm2.chrome.warmup" p.manager.logger.DebugfOp( debugOp, "allowing %v to startup", diff --git a/internal/pkg/pm2/unoconv.go b/internal/pkg/pm2/unoconv.go index 87319718..48ec164d 100644 --- a/internal/pkg/pm2/unoconv.go +++ b/internal/pkg/pm2/unoconv.go @@ -22,7 +22,7 @@ func (p *unoconv) Fullname() string { } func (p *unoconv) Start() error { - const op = "unoconv.Start" + const op = "pm2.unoconv.Start" if err := p.manager.start(p); err != nil { return &standarderror.Error{Op: op, Err: err} } @@ -30,7 +30,7 @@ func (p *unoconv) Start() error { } func (p *unoconv) Shutdown() error { - const op = "unoconv.Shutdown" + const op = "pm2.unoconv.Shutdown" if err := p.manager.shutdown(p); err != nil { return &standarderror.Error{Op: op, Err: err} } diff --git a/internal/pkg/standarderror/standarderror.go b/internal/pkg/standarderror/standarderror.go index cda9cae9..4391374e 100644 --- a/internal/pkg/standarderror/standarderror.go +++ b/internal/pkg/standarderror/standarderror.go @@ -82,7 +82,6 @@ func Message(err error) string { // Op returns the logical operation of the error, if available. // Otherwise returns an empty string. -// FIXME: "resource.ChromePrinterOptions: float64: : " func Op(err error) string { if err == nil { return "" @@ -93,12 +92,10 @@ func Op(err error) string { } var buf bytes.Buffer if e.Op != "" { - fmt.Fprintf(&buf, "%s: ", e.Op) + fmt.Fprintf(&buf, "%s", e.Op) } - if e.Err != nil { - if wrappedOp := Op(e.Err); wrappedOp != "" { - fmt.Fprintf(&buf, "%s: ", wrappedOp) - } + if nestedOp := Op(e.Err); nestedOp != "" { + fmt.Fprintf(&buf, ": %s", nestedOp) } return buf.String() }