From 1a27697dda3ebbaf6d384be0bf0b01958e4a8abc Mon Sep 17 00:00:00 2001 From: Dan Gottlieb Date: Tue, 21 Jan 2025 13:54:36 -0500 Subject: [PATCH] RSDK-9721: Improve firstRun error message for missing meta.json. (#4726) --- config/module.go | 36 ++++++++++++++++++++---------------- config/module_test.go | 2 +- utils/variables.go | 15 +++++++-------- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/config/module.go b/config/module.go index 7040e8a77ae..7548ec4a3c0 100644 --- a/config/module.go +++ b/config/module.go @@ -4,7 +4,7 @@ import ( "bufio" "context" "encoding/json" - stderrors "errors" + "errors" "fmt" "os" "os/exec" @@ -15,7 +15,6 @@ import ( "sync" "time" - "github.com/pkg/errors" goutils "go.viam.com/utils" "go.viam.com/rdk/logging" @@ -104,7 +103,7 @@ func (m *Module) validate(path string) error { if m.Type == ModuleTypeLocal { _, err := os.Stat(m.ExePath) if err != nil { - return errors.Wrapf(err, "module %s executable path error", path) + return fmt.Errorf("module %s executable path error: %w", path, err) } } @@ -113,7 +112,7 @@ func (m *Module) validate(path string) error { } if m.Name == reservedModuleName { - return errors.Errorf("module %s cannot use the reserved name of %s", path, reservedModuleName) + return fmt.Errorf("module %s cannot use the reserved name of %s", path, reservedModuleName) } return nil @@ -168,7 +167,7 @@ func (m Module) exeDir(packagesDir string) (string, error) { func parseJSONFile[T any](path string) (*T, error) { f, err := os.Open(path) //nolint:gosec if err != nil { - return nil, errors.Wrap(err, "reading json file") + return nil, fmt.Errorf("reading json file: %w", err) } var target T err = json.NewDecoder(f).Decode(&target) @@ -223,7 +222,7 @@ func (m Module) EvaluateExePath(packagesDir string) (string, error) { meta, err := parseJSONFile[JSONManifest](metaPath) if err != nil { // note: this error deprecates the side-by-side case because the side-by-side case is deprecated. - return "", errors.Wrapf(err, "couldn't find meta.json inside tarball %s (or next to it)", m.ExePath) + return "", fmt.Errorf("couldn't find meta.json inside tarball %s (or next to it): %w", m.ExePath, err) } entrypoint, err := utils.SafeJoinDir(exeDir, meta.Entrypoint) if err != nil { @@ -258,8 +257,13 @@ func (m *Module) FirstRun( return err } - // check if FirstRun already ran successfully for this module version by - // checking if a success marker file exists on disk. + // check if FirstRun already ran successfully for this module version by checking if a success + // marker file exists on disk. An example module directory structure: + // + // .viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_1_0-linux-amd64/ + // .viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_1_0-linux-amd64/bin/ + // .viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_1_0-linux-amd64/bin.first_run_succeeded + // .viam/packages/data/module/e76d1b3b-0468-4efd-bb7f-fb1d2b352fcb-viamrtsp-0_1_0-linux-amd64/bin/viamrtsp firstRunSuccessPath := unpackedModDir + FirstRunSuccessSuffix if _, err := os.Stat(firstRunSuccessPath); !errors.Is(err, os.ErrNotExist) { logger.Info("first run already ran") @@ -272,7 +276,7 @@ func (m *Module) FirstRun( var pathErr *os.PathError switch { case errors.As(err, &pathErr): - logger.Debugw("meta.json not found, skipping first run", "error", err) + logger.Infow("meta.json does not exist, skipping first run") return nil case err != nil: logger.Warnw("failed to parse meta.json, skipping first run", "error", err) @@ -404,7 +408,7 @@ func (m Module) getJSONManifest(unpackedModDir string, env map[string]string) (* if registryErr != nil { // return from getJSONManifest() if the error returned does NOT indicate that the file wasn't found if !os.IsNotExist(registryErr) { - return nil, "", errors.Wrap(registryErr, "registry module") + return nil, "", fmt.Errorf("registry module: %w", registryErr) } } @@ -425,10 +429,10 @@ func (m Module) getJSONManifest(unpackedModDir string, env map[string]string) (* if registryTarballErr != nil { if !os.IsNotExist(registryTarballErr) { if online { - return nil, "", errors.Wrap(registryTarballErr, "registry module") + return nil, "", fmt.Errorf("registry module: %w", registryTarballErr) } - return nil, "", errors.Wrap(registryTarballErr, "local tarball") + return nil, "", fmt.Errorf("local tarball: %w", registryTarballErr) } } @@ -449,7 +453,7 @@ func (m Module) getJSONManifest(unpackedModDir string, env map[string]string) (* meta, localTarballErr = findMetaJSONFile(exeDir) if localTarballErr != nil { if !os.IsNotExist(localTarballErr) { - return nil, "", errors.Wrap(localTarballErr, "local tarball") + return nil, "", fmt.Errorf("local tarball: %w", localTarballErr) } } @@ -460,14 +464,14 @@ func (m Module) getJSONManifest(unpackedModDir string, env map[string]string) (* if online { if !ok { - return nil, "", errors.Wrap(registryTarballErr, "registry module: failed to find meta.json. VIAM_MODULE_ROOT not set") + return nil, "", fmt.Errorf("registry module: failed to find meta.json. VIAM_MODULE_ROOT not set: %w", registryTarballErr) } - return nil, "", errors.Wrap(stderrors.Join(registryErr, registryTarballErr), "registry module: failed to find meta.json") + return nil, "", fmt.Errorf("registry module: failed to find meta.json: %w", errors.Join(registryErr, registryTarballErr)) } if !localNonTarball { - return nil, "", errors.Wrap(stderrors.Join(registryTarballErr, localTarballErr), "local tarball: failed to find meta.json") + return nil, "", fmt.Errorf("local tarball: failed to find meta.json: %w", errors.Join(registryTarballErr, localTarballErr)) } return nil, "", errors.New("local non-tarball: did not search for meta.json") diff --git a/config/module_test.go b/config/module_test.go index 8329dad5c23..19e40429670 100644 --- a/config/module_test.go +++ b/config/module_test.go @@ -117,7 +117,7 @@ func TestRegistryModuleFirstRun(t *testing.T) { err := module.FirstRun(ctx, localPackagesDir, dataDir, env, logger) test.That(t, err, test.ShouldBeNil) - test.That(t, observedLogs.FilterMessage("meta.json not found, skipping first run").Len(), test.ShouldEqual, 1) + test.That(t, observedLogs.FilterMessage("meta.json does not exist, skipping first run").Len(), test.ShouldEqual, 1) }) t.Run("MetaFileInvalid", func(t *testing.T) { diff --git a/utils/variables.go b/utils/variables.go index ea193bc4f22..43c0b5eae94 100644 --- a/utils/variables.go +++ b/utils/variables.go @@ -1,11 +1,10 @@ package utils import ( + "fmt" "reflect" "regexp" "strings" - - "github.com/pkg/errors" ) var ( @@ -24,10 +23,10 @@ var ( // ValidateResourceName validates that the resource follows our naming requirements. func ValidateResourceName(name string) error { if len(name) > 60 { - return errors.Errorf("name %q must be 60 characters or fewer", name) + return fmt.Errorf("name %q must be 60 characters or fewer", name) } if !validResourceNameRegex.MatchString(name) { - return errors.Errorf("name %q %s", name, validResourceNameExplanation) + return fmt.Errorf("name %q %s", name, validResourceNameExplanation) } return nil } @@ -37,10 +36,10 @@ func ValidateResourceName(name string) error { // accepts valid socket paths. func ValidateModuleName(name string) error { if len(name) > 200 { - return errors.Errorf("module name %q must be 200 characters or fewer", name) + return fmt.Errorf("module name %q must be 200 characters or fewer", name) } if !validResourceNameRegex.MatchString(name) { - return errors.Errorf("module name %q %s", name, validResourceNameExplanation) + return fmt.Errorf("module name %q %s", name, validResourceNameExplanation) } return nil } @@ -48,10 +47,10 @@ func ValidateModuleName(name string) error { // ValidatePackageName validates that the package follows our naming requirements. func ValidatePackageName(name string) error { if len(name) > 200 { - return errors.Errorf("package name %q must be 200 characters or fewer", name) + return fmt.Errorf("package name %q must be 200 characters or fewer", name) } if !validResourceNameRegex.MatchString(name) { - return errors.Errorf("package name %q %s", name, validResourceNameExplanation) + return fmt.Errorf("package name %q %s", name, validResourceNameExplanation) } return nil }