diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 8a9028aaa63..d4643773409 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/dashdiffs" "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/guardian" @@ -334,7 +333,7 @@ func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) Response { dashRedirect := dtos.DashboardRedirect{RedirectUri: url} return JSON(200, &dashRedirect) } - log.Warnf("Failed to get slug from database, %s", err.Error()) + hs.log.Warn("Failed to get slug from database", "err", err) } filePath := hs.Cfg.DefaultHomeDashboardPath @@ -346,7 +345,11 @@ func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) Response { if err != nil { return Error(500, "Failed to load home dashboard", err) } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + hs.log.Warn("Failed to close dashboard file", "path", filePath, "err", err) + } + }() dash := dtos.DashboardFullWithMeta{} dash.Meta.IsHome = true diff --git a/pkg/cmd/grafana-server/main.go b/pkg/cmd/grafana-server/main.go index b68a76238e8..0ab0a6a7186 100644 --- a/pkg/cmd/grafana-server/main.go +++ b/pkg/cmd/grafana-server/main.go @@ -116,7 +116,11 @@ func executeServer(configFile, homePath, pidFile, packaging string, traceDiagnos if err != nil { panic(err) } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Error("Failed to write trace diagnostics", "path", traceDiagnostics.file, "err", err) + } + }() if err := trace.Start(f); err != nil { panic(err) diff --git a/pkg/components/imguploader/azureblobuploader.go b/pkg/components/imguploader/azureblobuploader.go index 50f5165b4af..160e2b5119a 100644 --- a/pkg/components/imguploader/azureblobuploader.go +++ b/pkg/components/imguploader/azureblobuploader.go @@ -49,7 +49,11 @@ func (az *AzureBlobUploader) Upload(ctx context.Context, imageDiskPath string) ( if err != nil { return "", err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + az.log.Warn("Failed to close file", "path", imageDiskPath, "err", err) + } + }() randomFileName, err := util.GetRandomString(30) if err != nil { diff --git a/pkg/components/imguploader/gcs/gcsuploader.go b/pkg/components/imguploader/gcs/gcsuploader.go index a2fee45b580..e869623cb45 100644 --- a/pkg/components/imguploader/gcs/gcsuploader.go +++ b/pkg/components/imguploader/gcs/gcsuploader.go @@ -153,7 +153,11 @@ func (u *Uploader) uploadFile( if err != nil { return err } - defer fileReader.Close() + defer func() { + if err := fileReader.Close(); err != nil { + u.log.Warn("Failed to close file", "err", err, "path", imageDiskPath) + } + }() // Set public access if not generating a signed URL pubAcc := !u.enableSignedURLs diff --git a/pkg/components/imguploader/s3uploader.go b/pkg/components/imguploader/s3uploader.go index d49e9018f91..59ee5a11098 100644 --- a/pkg/components/imguploader/s3uploader.go +++ b/pkg/components/imguploader/s3uploader.go @@ -80,12 +80,17 @@ func (u *S3Uploader) Upload(ctx context.Context, imageDiskPath string) (string, if err != nil { return "", err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + u.log.Warn("Failed to close file", "path", imageDiskPath, "err", err) + } + }() sess, err = session.NewSession(cfg) if err != nil { return "", err } + uploader := s3manager.NewUploader(sess) result, err := uploader.UploadWithContext(ctx, &s3manager.UploadInput{ Bucket: aws.String(u.bucket), diff --git a/pkg/infra/log/log.go b/pkg/infra/log/log.go index 737d48941c3..ccec1058e9f 100644 --- a/pkg/infra/log/log.go +++ b/pkg/infra/log/log.go @@ -80,6 +80,10 @@ func Warnf(format string, v ...interface{}) { Root.Warn(message) } +func Error(msg string, args ...interface{}) { + Root.Error(msg, args...) +} + func Errorf(skip int, format string, v ...interface{}) { Root.Error(fmt.Sprintf(format, v...)) } diff --git a/pkg/plugins/dashboards.go b/pkg/plugins/dashboards.go index 30396bf2004..f2d533b1de1 100644 --- a/pkg/plugins/dashboards.go +++ b/pkg/plugins/dashboards.go @@ -91,7 +91,6 @@ func GetPluginDashboards(orgId int64, pluginId string) ([]*PluginDashboardInfoDT func loadPluginDashboard(pluginId, path string) (*models.Dashboard, error) { plugin, exists := Plugins[pluginId] - if !exists { return nil, PluginNotFoundError{pluginId} } @@ -102,7 +101,11 @@ func loadPluginDashboard(pluginId, path string) (*models.Dashboard, error) { return nil, err } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + plog.Warn("Failed to close file", "path", dashboardFilePath, "err", err) + } + }() data, err := simplejson.NewFromReader(reader) if err != nil { diff --git a/pkg/plugins/manifest.go b/pkg/plugins/manifest.go index 9b6e2cb00c4..b082dedf2d2 100644 --- a/pkg/plugins/manifest.go +++ b/pkg/plugins/manifest.go @@ -113,7 +113,11 @@ func getPluginSignatureState(log log.Logger, plugin *PluginBase) PluginSignature if err != nil { return PluginSignatureModified } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Warn("Failed to close plugin file", "path", fp, "err", err) + } + }() h := sha256.New() if _, err := io.Copy(h, f); err != nil { diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 0818a8dde05..77a73f71a3f 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -273,7 +273,11 @@ func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error { if err != nil { return err } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + scanner.log.Warn("Failed to close JSON file", "path", jsonFPath, "err", err) + } + }() jsonParser := json.NewDecoder(reader) @@ -343,7 +347,11 @@ func (s *PluginScanner) loadPlugin(pluginJSONFilePath string) error { if err != nil { return err } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + s.log.Warn("Failed to close JSON file", "path", pluginJSONFilePath, "err", err) + } + }() jsonParser := json.NewDecoder(reader) pluginCommon := PluginBase{} diff --git a/pkg/services/alerting/notifiers/discord.go b/pkg/services/alerting/notifiers/discord.go index 3801b91b286..a80d6ad429e 100644 --- a/pkg/services/alerting/notifiers/discord.go +++ b/pkg/services/alerting/notifiers/discord.go @@ -2,6 +2,7 @@ package notifiers import ( "bytes" + "fmt" "io" "mime/multipart" "os" @@ -168,12 +169,20 @@ func (dn *DiscordNotifier) embedImage(cmd *models.SendWebhookSync, imagePath str return err } } - - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + dn.log.Warn("Failed to close file", "path", imagePath, "err", err) + } + }() var b bytes.Buffer w := multipart.NewWriter(&b) - + defer func() { + if err := w.Close(); err != nil { + // Should be OK since we already close it on non-error path + dn.log.Warn("Failed to close multipart writer", "err", err) + } + }() fw, err := w.CreateFormField("payload_json") if err != nil { return err @@ -192,7 +201,9 @@ func (dn *DiscordNotifier) embedImage(cmd *models.SendWebhookSync, imagePath str return err } - w.Close() + if err := w.Close(); err != nil { + return fmt.Errorf("failed to close multipart writer: %w", err) + } cmd.Body = b.String() cmd.ContentType = w.FormDataContentType() diff --git a/pkg/services/alerting/notifiers/pushover.go b/pkg/services/alerting/notifiers/pushover.go index 79cc97520cc..c13270205bc 100644 --- a/pkg/services/alerting/notifiers/pushover.go +++ b/pkg/services/alerting/notifiers/pushover.go @@ -292,7 +292,11 @@ func (pn *PushoverNotifier) genPushoverBody(evalContext *alerting.EvalContext, m if err != nil { return nil, b, err } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + pn.log.Warn("Failed to close file", "path", evalContext.ImageOnDiskPath, "err", err) + } + }() fw, err := w.CreateFormFile("attachment", evalContext.ImageOnDiskPath) if err != nil { diff --git a/pkg/services/alerting/notifiers/slack.go b/pkg/services/alerting/notifiers/slack.go index 3ec33a30ffa..e7f7e4c804b 100644 --- a/pkg/services/alerting/notifiers/slack.go +++ b/pkg/services/alerting/notifiers/slack.go @@ -321,7 +321,7 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { return err } if sn.Token != "" && sn.UploadImage { - err = slackFileUpload(evalContext, sn.log, "https://slack.com/api/files.upload", sn.Recipient, sn.Token) + err = sn.slackFileUpload(evalContext, sn.log, "https://slack.com/api/files.upload", sn.Recipient, sn.Token) if err != nil { return err } @@ -329,12 +329,12 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { return nil } -func slackFileUpload(evalContext *alerting.EvalContext, log log.Logger, url string, recipient string, token string) error { +func (sn *SlackNotifier) slackFileUpload(evalContext *alerting.EvalContext, log log.Logger, url string, recipient string, token string) error { if evalContext.ImageOnDiskPath == "" { evalContext.ImageOnDiskPath = filepath.Join(setting.HomePath, "public/img/mixed_styles.png") } log.Info("Uploading to slack via file.upload API") - headers, uploadBody, err := generateSlackBody(evalContext.ImageOnDiskPath, token, recipient) + headers, uploadBody, err := sn.generateSlackBody(evalContext.ImageOnDiskPath, token, recipient) if err != nil { return err } @@ -346,37 +346,47 @@ func slackFileUpload(evalContext *alerting.EvalContext, log log.Logger, url stri return nil } -func generateSlackBody(file string, token string, recipient string) (map[string]string, bytes.Buffer, error) { +func (sn *SlackNotifier) generateSlackBody(path string, token string, recipient string) (map[string]string, bytes.Buffer, error) { // Slack requires all POSTs to files.upload to present // an "application/x-www-form-urlencoded" encoded querystring // See https://api.slack.com/methods/files.upload var b bytes.Buffer w := multipart.NewWriter(&b) + defer func() { + if err := w.Close(); err != nil { + // Shouldn't matter since we already close w explicitly on the non-error path + sn.log.Warn("Failed to close multipart writer", "err", err) + } + }() + // Add the generated image file - f, err := os.Open(file) + f, err := os.Open(path) if err != nil { return nil, b, err } - defer f.Close() - fw, err := w.CreateFormFile("file", file) + defer func() { + if err := f.Close(); err != nil { + sn.log.Warn("Failed to close file", "path", path, "err", err) + } + }() + fw, err := w.CreateFormFile("file", path) if err != nil { return nil, b, err } - _, err = io.Copy(fw, f) - if err != nil { + if _, err := io.Copy(fw, f); err != nil { return nil, b, err } // Add the authorization token - err = w.WriteField("token", token) - if err != nil { + if err := w.WriteField("token", token); err != nil { return nil, b, err } // Add the channel(s) to POST to - err = w.WriteField("channels", recipient) - if err != nil { + if err := w.WriteField("channels", recipient); err != nil { return nil, b, err } - w.Close() + if err := w.Close(); err != nil { + return nil, b, fmt.Errorf("failed to close multipart writer: %w", err) + } headers := map[string]string{ "Content-Type": w.FormDataContentType(), "Authorization": "auth_token=\"" + token + "\"", diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index 709dba0bb3f..bbf4c0d7739 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -347,7 +347,11 @@ func (fr *FileReader) readDashboardFromFile(path string, lastModified time.Time, if err != nil { return nil, err } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + fr.log.Warn("Failed to close file", "path", path, "err", err) + } + }() all, err := ioutil.ReadAll(reader) if err != nil { diff --git a/pkg/services/rendering/http_mode.go b/pkg/services/rendering/http_mode.go index 23bb912a75f..7cc30c891a6 100644 --- a/pkg/services/rendering/http_mode.go +++ b/pkg/services/rendering/http_mode.go @@ -96,7 +96,12 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, renderKey string, if err != nil { return nil, err } - defer out.Close() + defer func() { + if err := out.Close(); err != nil { + // We already close the file explicitly in the non-error path, so shouldn't be a problem + rs.log.Warn("Failed to close file", "path", filePath, "err", err) + } + }() _, err = io.Copy(out, resp.Body) if err != nil { // check that we didn't timeout while receiving the response. @@ -107,6 +112,9 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, renderKey string, rs.log.Error("Remote rendering request failed", "error", err) return nil, fmt.Errorf("remote rendering request failed: %w", err) } + if err := out.Close(); err != nil { + return nil, fmt.Errorf("failed to write to %q: %w", filePath, err) + } return &RenderResult{FilePath: filePath}, err }