From d138afaf32ecafd3ea1fd3a919acca1266493fec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 18 Feb 2024 09:37:05 +0000 Subject: [PATCH] don't panic when we can error as easily Panicking in small helpers or in funcs that don't return error has proved useful to keep code easier to maintain, particularly for cases that should typically never happen. However, in these cases we can error just as easily. In particular, I was getting a panic whenever I forgot that I was running garble with Go master (1.23), which is over the top. --- internal/linker/linker.go | 2 +- main.go | 8 ++++---- shared.go | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/linker/linker.go b/internal/linker/linker.go index 98df0917..dccba869 100644 --- a/internal/linker/linker.go +++ b/internal/linker/linker.go @@ -217,7 +217,7 @@ func PatchLinker(goRoot, goVersion, cacheDir, tempDir string) (string, func(), e patchesVer, modFiles, patches, err := loadLinkerPatches(majorGoVersion) if err != nil { - panic(fmt.Errorf("cannot retrieve linker patches: %v", err)) + return "", nil, fmt.Errorf("cannot retrieve linker patches: %v", err) } outputLinkPath, err := cachePath(cacheDir) diff --git a/main.go b/main.go index 60d1b2b6..cd75b1e3 100644 --- a/main.go +++ b/main.go @@ -200,7 +200,7 @@ func (w *uniqueLineWriter) Write(p []byte) (n int, err error) { panic("unexpected use of uniqueLineWriter with -debug unset") } if bytes.Count(p, []byte("\n")) != 1 { - panic(fmt.Sprintf("log write wasn't just one line: %q", p)) + return 0, fmt.Errorf("log write wasn't just one line: %q", p) } if w.seen[string(p)] { return len(p), nil @@ -1262,7 +1262,7 @@ func (tf *transformer) processImportCfg(flags []string, requiredPkgs []string) ( beforePath, afterPath := pair[0], pair[1] lpkg, err := listPackage(tf.curPkg, beforePath) if err != nil { - panic(err) // shouldn't happen + return "", err } if lpkg.ToObfuscate { // Note that beforePath is not the canonical path. @@ -1327,7 +1327,7 @@ func (tf *transformer) processImportCfg(flags []string, requiredPkgs []string) ( if strings.HasSuffix(tf.curPkg.ImportPath, ".test]") && strings.HasPrefix(tf.curPkg.ImportPath, impPath) { continue } - panic(err) // shouldn't happen + return "", err } if lpkg.Name != "main" { impPath = lpkg.obfuscatedImportPath() @@ -1470,7 +1470,7 @@ func computePkgCache(fsCache *cache.Cache, lpkg *listedPackage, pkg *types.Packa // Shadowing lpkg ensures we don't use the wrong listedPackage below. lpkg, err := listPackage(lpkg, imp) if err != nil { - panic(err) // shouldn't happen + return computed, err } if lpkg.BuildID == "" { continue // nothing to load diff --git a/shared.go b/shared.go index bcadff0e..13546b98 100644 --- a/shared.go +++ b/shared.go @@ -379,7 +379,7 @@ func listPackage(from *listedPackage, path string) (*listedPackage, error) { return pkg, nil } if listedRuntimeLinknamed { - panic(fmt.Sprintf("package %q still missing after go list call", path)) + return nil, fmt.Errorf("package %q still missing after go list call", path) } startTime := time.Now() missing := make([]string, 0, len(runtimeLinknamed)) @@ -397,11 +397,11 @@ func listPackage(from *listedPackage, path string) (*listedPackage, error) { } // We don't need any information about their dependencies, in this case. if err := appendListedPackages(missing, false); err != nil { - panic(err) // should never happen + return nil, fmt.Errorf("failed to load missing runtime-linknamed packages: %v", err) } pkg, ok := sharedCache.ListedPackages[path] if !ok { - panic(fmt.Sprintf("std listed another std package that we can't find: %s", path)) + return nil, fmt.Errorf("std listed another std package that we can't find: %s", path) } listedRuntimeLinknamed = true log.Printf("listed %d missing runtime-linknamed packages in %s", len(missing), debugSince(startTime))