Skip to content

Commit

Permalink
don't panic when we can error as easily
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mvdan authored and pagran committed Feb 18, 2024
1 parent 975f608 commit d138afa
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
2 changes: 1 addition & 1 deletion internal/linker/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down

0 comments on commit d138afa

Please sign in to comment.