From 50d25fe7b7c57d33a0e8559be366d3a0e6fffd7d Mon Sep 17 00:00:00 2001 From: Thomas von Dein Date: Wed, 29 Mar 2023 14:59:04 +0200 Subject: [PATCH] fix error handling --- api/auth.go | 3 +++ api/cleaner.go | 8 +++----- api/db_test.go | 11 +++++++++-- api/fileio.go | 22 +++++++++++++++++++--- api/form_handlers.go | 6 +++++- api/upload_handlers.go | 27 +++++++++++++++++++++------ cfg/config.go | 4 ++-- cmd/root.go | 32 ++++++++++++++++++-------------- ephemerup.hcl | 2 +- upctl/cmd/formcommands.go | 3 +-- upctl/cmd/root.go | 13 ++++++++----- upctl/lib/client.go | 10 ++++++---- upctl/lib/output.go | 2 -- upctl/t/t1 | 1 - 14 files changed, 96 insertions(+), 48 deletions(-) delete mode 100644 upctl/t/t1 diff --git a/api/auth.go b/api/auth.go index 6602be2..afc79c1 100644 --- a/api/auth.go +++ b/api/auth.go @@ -71,6 +71,9 @@ func AuthValidateOnetimeKey(c *fiber.Ctx, key string, db *Db) (bool, error) { } sess, err := Sessionstore.Get(c) + if err != nil { + return false, errors.New("Could not retrieve session from Sessionstore: " + err.Error()) + } // store the result into the session, the 'formid' key tells the // upload handler that the apicontext it sees is in fact a form id diff --git a/api/cleaner.go b/api/cleaner.go index c769a1c..dc97710 100644 --- a/api/cleaner.go +++ b/api/cleaner.go @@ -58,10 +58,6 @@ func DeleteExpiredUploads(conf *cfg.Config, db *Db) error { return err }) - if err != nil { - Log("DB error: %s", err.Error()) - } - return err } @@ -74,7 +70,9 @@ func BackgroundCleaner(conf *cfg.Config, db *Db) chan bool { for { select { case <-ticker.C: - DeleteExpiredUploads(conf, db) + if err := DeleteExpiredUploads(conf, db); err != nil { + Log("Failed to delete eypired uploads: %s", err.Error()) + } case <-done: ticker.Stop() return diff --git a/api/db_test.go b/api/db_test.go index 571a08e..c2dc66f 100644 --- a/api/db_test.go +++ b/api/db_test.go @@ -112,6 +112,10 @@ func TestDboperation(t *testing.T) { if tt.upload.Id != "" { // set ts ts, err := time.Parse(timeformat, tt.ts) + if err != nil { + t.Errorf("Could not parse time: " + err.Error()) + } + tt.upload.Created = common.Timestamp{Time: ts} // create new upload db object @@ -162,7 +166,7 @@ func TestDboperation(t *testing.T) { } // fetch again, shall return empty - response, err = db.Get(tt.context, tt.id, common.TypeUpload) + _, err = db.Get(tt.context, tt.id, common.TypeUpload) if err == nil { t.Errorf("Could fetch upload object again although we deleted it") } @@ -171,6 +175,9 @@ func TestDboperation(t *testing.T) { if tt.form.Id != "" { // set ts ts, err := time.Parse(timeformat, tt.ts) + if err != nil { + t.Errorf("Could not parse time: " + err.Error()) + } tt.form.Created = common.Timestamp{Time: ts} // create new form db object @@ -221,7 +228,7 @@ func TestDboperation(t *testing.T) { } // fetch again, shall return empty - response, err = db.Get(tt.context, tt.id, common.TypeForm) + _, err = db.Get(tt.context, tt.id, common.TypeForm) if err == nil { t.Errorf("Could fetch form object again although we deleted it") } diff --git a/api/fileio.go b/api/fileio.go index 8301e33..685cc71 100644 --- a/api/fileio.go +++ b/api/fileio.go @@ -114,17 +114,23 @@ func ZipDir(directory, zipfilename string) error { var wg sync.WaitGroup wg.Add(1) + failure := make(chan string) + // don't chdir the server itself go func() { defer wg.Done() - os.Chdir(directory) + if err := os.Chdir(directory); err != nil { + failure <- "Failed to changedir: " + err.Error() + return + } newDir, err := os.Getwd() if err != nil { + failure <- "Failed to get cwd: " + err.Error() } + if newDir != directory { - err = errors.New("Failed to changedir!") - return + failure <- "Failed to changedir!" } err = filepath.Walk(".", func(path string, info os.FileInfo, err error) error { @@ -171,9 +177,19 @@ func ZipDir(directory, zipfilename string) error { _, err = io.Copy(headerWriter, f) return err }) + + if err != nil { + failure <- "Failed to zip directory: " + err.Error() + } }() wg.Wait() + goterr := <-failure + + if goterr != "" { + return errors.New(goterr) + } + return err } diff --git a/api/form_handlers.go b/api/form_handlers.go index d55f244..c4f42ba 100644 --- a/api/form_handlers.go +++ b/api/form_handlers.go @@ -82,7 +82,11 @@ func FormCreate(c *fiber.Ctx, cfg *cfg.Config, db *Db) error { Log("Form created with API-Context %s", entry.Context) // we do this in the background to not thwart the server - go db.Insert(id, entry) + go func() { + if err := db.Insert(id, entry); err != nil { + Log("Failed to insert: " + err.Error()) + } + }() // everything went well so far res := &common.Response{Forms: []*common.Form{entry}} diff --git a/api/upload_handlers.go b/api/upload_handlers.go index 46f13d8..e6a5fef 100644 --- a/api/upload_handlers.go +++ b/api/upload_handlers.go @@ -53,7 +53,10 @@ func UploadPost(c *fiber.Ctx, cfg *cfg.Config, db *Db) error { var returnUrl string var formdata Meta - os.MkdirAll(filepath.Join(cfg.StorageDir, id), os.ModePerm) + if err := os.MkdirAll(filepath.Join(cfg.StorageDir, id), os.ModePerm); err != nil { + return JsonStatus(c, fiber.StatusInternalServerError, + "Unable to initialize directories: "+err.Error()) + } // fetch auxiliary form data form, err := c.MultipartForm() @@ -114,7 +117,11 @@ func UploadPost(c *fiber.Ctx, cfg *cfg.Config, db *Db) error { Log("Uploaded with API-Context %s", entry.Context) // we do this in the background to not thwart the server - go db.Insert(id, entry) + go func() { + if err := db.Insert(id, entry); err != nil { + Log("Failed to insert: " + err.Error()) + } + }() // everything went well so far res := &common.Response{Uploads: []*common.Upload{entry}} @@ -131,7 +138,9 @@ func UploadPost(c *fiber.Ctx, cfg *cfg.Config, db *Db) error { if err == nil { if len(r.Forms) == 1 { if r.Forms[0].Expire == "asap" { - db.Delete(apicontext, formid) + if err := db.Delete(apicontext, formid); err != nil { + Log("Failed to delete formid %s: %s", formid, err.Error()) + } } // email notification to form creator @@ -184,7 +193,11 @@ func UploadFetch(c *fiber.Ctx, cfg *cfg.Config, db *Db, shallExpire ...bool) err if _, err := os.Stat(filename); err != nil { // db entry is there, but file isn't (anymore?) - go db.Delete(apicontext, id) + go func() { + if err := db.Delete(apicontext, id); err != nil { + Log("Unable to delete entry id %s: %s", id, err.Error()) + } + }() return fiber.NewError(404, "No download with that id could be found!") } @@ -192,12 +205,14 @@ func UploadFetch(c *fiber.Ctx, cfg *cfg.Config, db *Db, shallExpire ...bool) err err = c.Download(filename, file) if len(shallExpire) > 0 { - if shallExpire[0] == true { + if shallExpire[0] { go func() { // check if we need to delete the file now and do it in the background if upload.Expire == "asap" { cleanup(filepath.Join(cfg.StorageDir, id)) - db.Delete(apicontext, id) + if err := db.Delete(apicontext, id); err != nil { + Log("Unable to delete entry id %s: %s", id, err.Error()) + } } }() } diff --git a/cfg/config.go b/cfg/config.go index dfbf732..3b1b761 100644 --- a/cfg/config.go +++ b/cfg/config.go @@ -62,10 +62,10 @@ type Config struct { Network string // only settable via config - Apicontexts []Apicontext `koanf:"apicontext"` + Apicontexts []Apicontext `koanf:"apicontexts"` // smtp settings - Mail Mailsettings `koanf:mail` + Mail Mailsettings `koanf:"mail"` // Internals only RegNormalizedFilename *regexp.Regexp diff --git a/cmd/root.go b/cmd/root.go index 8834071..0f5cd3a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -75,7 +75,9 @@ func Execute() error { f.StringVarP(&conf.AppName, "appname", "n", "ephemerupd "+conf.GetVersion(), "App name to say hi as") f.IntVarP(&conf.BodyLimit, "bodylimit", "b", 10250000000, "Max allowed upload size in bytes") - f.Parse(os.Args[1:]) + if err := f.Parse(os.Args[1:]); err != nil { + return err + } // exclude -6 and -4 if conf.V4only && conf.V6only { @@ -86,19 +88,19 @@ func Execute() error { var k = koanf.New(".") // Load the config files provided in the commandline or the default locations - configfiles := []string{} + var configfiles []string configfile, _ := f.GetString("config") if configfile != "" { configfiles = []string{configfile} } else { configfiles = []string{ - "/etc/ephemerupd.hcl", "/usr/local/etc/ephemerupd.hcl", // unix variants - filepath.Join(os.Getenv("HOME"), ".config", "ephemerupd", "ephemerupd.hcl"), - filepath.Join(os.Getenv("HOME"), ".ephemerupd"), - "ephemerupd.hcl", + "/etc/ephemerup.hcl", "/usr/local/etc/ephemerup.hcl", // unix variants + filepath.Join(os.Getenv("HOME"), ".config", "ephemerup", "ephemerup.hcl"), + filepath.Join(os.Getenv("HOME"), ".ephemerup"), + "ephemerup.hcl", } } - + repr.Print(configfiles) for _, cfgfile := range configfiles { if _, err := os.Stat(cfgfile); err == nil { if err := k.Load(file.Provider(cfgfile), hcl.Parser(true)); err != nil { @@ -109,10 +111,12 @@ func Execute() error { } // env overrides config file - k.Load(env.Provider("EPHEMERUPD_", ".", func(s string) string { + if err := k.Load(env.Provider("EPHEMERUPD_", ".", func(s string) string { return strings.Replace(strings.ToLower( strings.TrimPrefix(s, "EPHEMERUPD_")), "_", ".", -1) - }), nil) + }), nil); err != nil { + return errors.New("error loading environment: " + err.Error()) + } // command line overrides env if err := k.Load(posflag.Provider(f, ".", k), nil); err != nil { @@ -120,7 +124,9 @@ func Execute() error { } // fetch values - k.Unmarshal("", &conf) + if err := k.Unmarshal("", &conf); err != nil { + return errors.New("error unmarshalling: " + err.Error()) + } // there may exist some api context variables GetApicontextsFromEnv(&conf) @@ -180,7 +186,7 @@ func Execute() error { eg: EPHEMERUPD_CONTEXT_SUPPORT="support:tymag-fycyh-gymof-dysuf-doseb-puxyx" - ^^^^^^^- doesn't matter. + ^^^^^^^- doesn't matter. Modifies cfg.Config directly */ @@ -197,9 +203,7 @@ func GetApicontextsFromEnv(conf *cfg.Config) { } } - for _, ap := range conf.Apicontexts { - contexts = append(contexts, ap) - } + contexts = append(contexts, conf.Apicontexts...) conf.Apicontexts = contexts } diff --git a/ephemerup.hcl b/ephemerup.hcl index 274f3f0..69be05c 100644 --- a/ephemerup.hcl +++ b/ephemerup.hcl @@ -2,7 +2,7 @@ listen = ":8080" bodylimit = 10000 -apicontext = [ +apicontexts = [ { context = "root" key = "0fddbff5d8010f81cd28a7d77f3e38981b13d6164c2fd6e1c3f60a4287630c37", diff --git a/upctl/cmd/formcommands.go b/upctl/cmd/formcommands.go index b95d192..a140cfd 100644 --- a/upctl/cmd/formcommands.go +++ b/upctl/cmd/formcommands.go @@ -33,8 +33,7 @@ func FormCommand(conf *cfg.Config) *cobra.Command { // errors at this stage do not cause the usage to be shown //cmd.SilenceUsage = true if len(args) == 0 { - cmd.Help() - os.Exit(0) + return cmd.Help() } return nil }, diff --git a/upctl/cmd/root.go b/upctl/cmd/root.go index b2e5b46..7e52e52 100644 --- a/upctl/cmd/root.go +++ b/upctl/cmd/root.go @@ -130,13 +130,12 @@ func initConfig(cmd *cobra.Command, cfg *cfg.Config) error { v.SetEnvPrefix("upctl") // map flags to viper - bindFlags(cmd, v) - - return nil + return bindFlags(cmd, v) } // bind flags to viper settings (env+cfgfile) -func bindFlags(cmd *cobra.Command, v *viper.Viper) { +func bindFlags(cmd *cobra.Command, v *viper.Viper) error { + var fail error cmd.Flags().VisitAll(func(f *pflag.Flag) { // map flag name to config variable configName := f.Name @@ -144,7 +143,11 @@ func bindFlags(cmd *cobra.Command, v *viper.Viper) { // use config variable if flag is not set and config is set if !f.Changed && v.IsSet(configName) { val := v.Get(configName) - cmd.Flags().Set(f.Name, fmt.Sprintf("%v", val)) + if err := cmd.Flags().Set(f.Name, fmt.Sprintf("%v", val)); err != nil { + fail = err + } } }) + + return fail } diff --git a/upctl/lib/client.go b/upctl/lib/client.go index 2850b53..93483f8 100644 --- a/upctl/lib/client.go +++ b/upctl/lib/client.go @@ -181,7 +181,9 @@ func UploadFiles(w io.Writer, c *cfg.Config, args []string) error { var left float64 rq.R.SetUploadCallbackWithInterval(func(info req.UploadInfo) { left = float64(info.UploadedSize) / float64(info.FileSize) * 100.0 - bar.Add(int(left)) + if err := bar.Add(int(left)); err != nil { + fmt.Print("\r") + } }, 10*time.Millisecond) } @@ -278,7 +280,9 @@ func Download(w io.Writer, c *cfg.Config, args []string) error { callback := func(info req.DownloadInfo) { if info.Response.Response != nil { - bar.Add(1) + if err := bar.Add(1); err != nil { + fmt.Print("\r") + } } } @@ -344,6 +348,4 @@ func CreateForm(w io.Writer, c *cfg.Config) error { } return RespondExtended(w, resp) - - return nil } diff --git a/upctl/lib/output.go b/upctl/lib/output.go index 63c8861..8f01620 100644 --- a/upctl/lib/output.go +++ b/upctl/lib/output.go @@ -38,8 +38,6 @@ func prepareExpire(expire string, start common.Timestamp) string { return time.Unix(start.Unix()+int64(common.Duration2int(expire)), 0). Format("2006-01-02 15:04:05") } - - return "" } // generic table writer diff --git a/upctl/t/t1 b/upctl/t/t1 deleted file mode 100644 index 7c0bf85..0000000 --- a/upctl/t/t1 +++ /dev/null @@ -1 +0,0 @@ -Mon Mar 20 12:16:26 PM CET 2023