internal: handle 290s on 404
When a path 404s but the top level module exists, return the relevant
response text.
Also update the response text for clarity.
Change-Id: I9425355daf4ad3f88c58261cc6f778f79f0fc003
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/284585
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/frontend/404.go b/internal/frontend/404.go
index d213b16..11cd54c 100644
--- a/internal/frontend/404.go
+++ b/internal/frontend/404.go
@@ -171,13 +171,17 @@
status: http.StatusFound,
}, nil
}
- vms, err := db.GetVersionMapsNon2xxStatus(ctx, paths, requestedVersion)
+ vms, err := db.GetVersionMaps(ctx, paths, requestedVersion)
if err != nil {
return nil, err
}
var fetchResults []*fetchResult
for _, vm := range vms {
- fetchResults = append(fetchResults, fetchResultFromVersionMap(vm))
+ fr := fetchResultFromVersionMap(vm)
+ fetchResults = append(fetchResults, fr)
+ if vm.Status == http.StatusOK || vm.Status == 290 {
+ fr.err = errPathDoesNotExistInModule
+ }
}
if len(fetchResults) == 0 {
return nil, derrors.NotFound
diff --git a/internal/frontend/fetch.go b/internal/frontend/fetch.go
index 40f1eb8..5fec5be 100644
--- a/internal/frontend/fetch.go
+++ b/internal/frontend/fetch.go
@@ -280,11 +280,12 @@
fr := results[0]
if moduleMatchingPathPrefix != "" {
t := template.Must(template.New("").Parse(`{{.}}`))
- h, err := t.ExecuteToHTML(fmt.Sprintf("Package %s could not be found, but you can view module “%s” at <a href='https://pkg.go.dev/%s'>pkg.go.dev/%s</a>.",
+ h, err := t.ExecuteToHTML(fmt.Sprintf(`
+ <div class="Error-message">%s could not be found.</div>
+ <div class="Error-message">However, you can view <a href="https://pkg.go.dev/%s">module %s</a>.</div>`,
displayPath(fullPath, requestedVersion),
displayPath(moduleMatchingPathPrefix, requestedVersion),
displayPath(moduleMatchingPathPrefix, requestedVersion),
- displayPath(moduleMatchingPathPrefix, requestedVersion),
))
if err != nil {
fr.status = http.StatusInternalServerError
diff --git a/internal/frontend/fetch_test.go b/internal/frontend/fetch_test.go
index 3cb53ec..7b9fe67 100644
--- a/internal/frontend/fetch_test.go
+++ b/internal/frontend/fetch_test.go
@@ -116,12 +116,14 @@
wantErrorMessage: "Bad Request",
},
{
- name: "module found but package does not exist",
- modulePath: testModulePath,
- fullPath: "github.com/module/pkg-nonexistent",
- version: internal.LatestVersion,
- want: http.StatusNotFound,
- wantErrorMessage: "Package github.com/module/pkg-nonexistent could not be found, but you can view module “github.com/module” at <a href='https://pkg.go.dev/github.com/module'>pkg.go.dev/github.com/module</a>.",
+ name: "module found but package does not exist",
+ modulePath: testModulePath,
+ fullPath: "github.com/module/pkg-nonexistent",
+ version: internal.LatestVersion,
+ want: http.StatusNotFound,
+ wantErrorMessage: `
+ <div class="Error-message">github.com/module/pkg-nonexistent could not be found.</div>
+ <div class="Error-message">However, you can view <a href="https://pkg.go.dev/github.com/module">module github.com/module</a>.</div>`,
},
{
name: "module exists but fetch timed out",
@@ -148,10 +150,9 @@
t.Fatalf("fetchAndPoll(ctx, testDB, q, %q, %q, %q): %d; want = %d",
test.modulePath, test.fullPath, test.version, got, test.want)
}
-
if err != test.wantErrorMessage {
- t.Fatalf("fetchAndPoll(ctx, testDB, q, %q, %q, %q): %d; wantErrorMessage = %s",
- test.modulePath, test.fullPath, test.version, got, test.wantErrorMessage)
+ t.Fatalf("fetchAndPoll(ctx, testDB, q, %q, %q, %q): %d;\ngot = \n%q,\nwantErrorMessage = \n%q",
+ test.modulePath, test.fullPath, test.version, got, err, test.wantErrorMessage)
}
})
}
diff --git a/internal/postgres/version_map.go b/internal/postgres/version_map.go
index 8fdaa4c..4e759b2 100644
--- a/internal/postgres/version_map.go
+++ b/internal/postgres/version_map.go
@@ -94,9 +94,9 @@
}
}
-// GetVersionMapsNon2xxStatus returns all of the version maps for the provided
+// GetVersionMaps returns all of the version maps for the provided
// path and requested version if they are present.
-func (db *DB) GetVersionMapsNon2xxStatus(ctx context.Context, paths []string, requestedVersion string) (_ []*internal.VersionMap, err error) {
+func (db *DB) GetVersionMaps(ctx context.Context, paths []string, requestedVersion string) (_ []*internal.VersionMap, err error) {
defer derrors.Wrap(&err, "DB.GetVersionMapsWith4xxStatus(ctx, %v, %q)", paths, requestedVersion)
var result []*internal.VersionMap
@@ -117,7 +117,6 @@
q, args, err := versionMapSelect().
Where("module_path = ANY(?)", pq.Array(paths)).
Where(squirrel.Or{squirrel.Eq{"requested_version": requestedVersion}, squirrel.Eq{"resolved_version": requestedVersion}}).
- Where(squirrel.GtOrEq{"status": 400}).
OrderBy("module_path DESC").
PlaceholderFormat(squirrel.Dollar).ToSql()
if err != nil {
diff --git a/internal/postgres/version_map_test.go b/internal/postgres/version_map_test.go
index 3e50637..d4d5347 100644
--- a/internal/postgres/version_map_test.go
+++ b/internal/postgres/version_map_test.go
@@ -99,9 +99,7 @@
want := map[string]bool{}
for _, test := range tests {
paths = append(paths, test.path)
- if test.status >= 400 {
- want[test.path] = true
- }
+ want[test.path] = true
if err := testDB.UpsertVersionMap(ctx, &internal.VersionMap{
ModulePath: test.path,
RequestedVersion: internal.LatestVersion,
@@ -112,7 +110,7 @@
t.Fatal(err)
}
}
- vms, err := testDB.GetVersionMapsNon2xxStatus(ctx, paths, internal.LatestVersion)
+ vms, err := testDB.GetVersionMaps(ctx, paths, internal.LatestVersion)
if err != nil {
t.Fatal(err)
}
