Skip to content

Commit

Permalink
internal/fetch: report DocTooLarge error in goPackage.err
Browse files Browse the repository at this point in the history
loadPackage has two return values: a *goPackage and an error.

Before this CL, if a package's documentation was too large,
loadPackage would return the package and a non-nil error.

With this CL, that error is stored in the goPackage struct in a new
field, and loadPackage returns a nil error.

Aside from being more idiomatic, this change will enable future
changes needed to support multiple GOOS/GOARCH combinations.

For golang/go#37232

Change-Id: If73adde76239046eb22409659d6795c09bbb1fd4
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/279457
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
  • Loading branch information
jba committed Dec 22, 2020
1 parent f5ca8b0 commit b248e48
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
12 changes: 6 additions & 6 deletions internal/fetch/load.go
Expand Up @@ -59,7 +59,7 @@ var goEnvs = []struct{ GOOS, GOARCH string }{
// loadPackage returns nil, nil.
//
// If the package is fine except that its documentation is too large, loadPackage
// returns both a package and a non-nil error with godoc.ErrTooLarge in its chain.
// returns a package whose err field is a non-nil error with godoc.ErrTooLarge in its chain.
func loadPackage(ctx context.Context, zipGoFiles []*zip.File, innerPath string, sourceInfo *source.Info, modInfo *godoc.ModuleInfo) (_ *goPackage, err error) {
defer derrors.Wrap(&err, "loadPackage(ctx, zipGoFiles, %q, sourceInfo, modInfo)", innerPath)
ctx, span := trace.StartSpan(ctx, "fetch.loadPackage")
Expand All @@ -70,7 +70,8 @@ func loadPackage(ctx context.Context, zipGoFiles []*zip.File, innerPath string,
return nil, err
}
if pkg != nil {
return pkg, err
pkg.err = err
return pkg, nil
}
}
return nil, nil
Expand All @@ -88,9 +89,9 @@ var httpPost = http.Post
// zipGoFiles must contain only .go files that have been verified
// to be of reasonable size.
//
// The returned Package.Licenses field is not populated.
// The returned goPackage.licenses field is not populated.
//
// It returns a nil Package if the directory doesn't contain a Go package
// It returns a nil goPackage if the directory doesn't contain a Go package
// or all .go files have been excluded by constraints.
// A *BadPackageError error is returned if the directory
// contains .go files but do not make up a valid package.
Expand All @@ -115,8 +116,7 @@ func loadPackageWithBuildContext(ctx context.Context, goos, goarch string, zipGo
docPkg.AddFile(pf, removeNodes)
}

// Encode before rendering: both operations mess with the AST, but Encode restores
// it enough to make Render work.
// Encode first, because Render messes with the AST.
src, err := docPkg.Encode(ctx)
if err != nil {
return nil, err
Expand Down
9 changes: 5 additions & 4 deletions internal/fetch/package.go
Expand Up @@ -43,6 +43,7 @@ type goPackage struct {
// series.
v1path string
source []byte // the source files of the package, for generating doc at serving time
err error // non-fatal error when loading the package (e.g. documentation is too large)
}

// extractPackagesFromZip returns a slice of packages from the module zip r.
Expand Down Expand Up @@ -197,13 +198,9 @@ func extractPackagesFromZip(ctx context.Context, modulePath, resolvedVersion str
incompleteDirs[innerPath] = true
status = derrors.PackageInvalidContents
errMsg = err.Error()
} else if errors.Is(err, godoc.ErrTooLarge) {
status = derrors.PackageDocumentationHTMLTooLarge
errMsg = err.Error()
} else if err != nil {
return nil, nil, fmt.Errorf("unexpected error loading package: %v", err)
}

var pkgPath string
if pkg == nil {
// No package.
Expand All @@ -214,6 +211,10 @@ func extractPackagesFromZip(ctx context.Context, modulePath, resolvedVersion str
}
pkgPath = path.Join(modulePath, innerPath)
} else {
if errors.Is(pkg.err, godoc.ErrTooLarge) {
status = derrors.PackageDocumentationHTMLTooLarge
errMsg = pkg.err.Error()
}
if d != nil { // should only be nil for tests
isRedist, lics := d.PackageInfo(innerPath)
pkg.isRedistributable = isRedist
Expand Down

0 comments on commit b248e48

Please sign in to comment.