mod/modregistry: treat all 404 errors as not-found

Despite the fact that the OCI specification explicitly lists the error
codes that a registry should return, it seems that some registries do
not follow the spec.

Given that 404 is a reasonably unambiguous error, it seems OK to treat
all 404 errors as if they were returning one of the standard not-found
errors.

Also change `GetModule` so that it also treats "not found" responses
in the same way as the `ModuleVersions` call.

Fixes #2982

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I96a4e28f38ad1fcf78c0be70a6aaa38713641123
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194495
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: rogpeppe

cmd/cue/cmd/testdata/script/registry_publish.txtar

@@ -13,7 +13,7 @@ cmp stdout ../expect-eval-stdout
 env CUE_REGISTRY=$ORIG_CUE_REGISTRY
 env CUE_CACHE_DIR=$WORK/.tmp/different-cache
 ! exec cue eval
-stderr 'repository name not known to registry'
+stderr 'cannot fetch [email protected]: module [email protected]: module not found'
 
 -- expect-eval-stdout --
 main:                   "main"

cmd/cue/cmd/testdata/script/registry_wrong_prefix.txtar

@@ -3,7 +3,7 @@ env CUE_REGISTRY=$DEBUG_REGISTRY_HOST/something+insecure
 cmp stderr expect-stderr
 
 -- expect-stderr --
-import failed: cannot find package "example.com/e": cannot fetch example.com/[email protected]: module example.com/[email protected]: 404 Not Found: name unknown: repository name not known to registry:
+import failed: cannot find package "example.com/e": cannot fetch example.com/[email protected]: module example.com/[email protected]: module not found:
     ./main.cue:2:8
 -- main.cue --
 package main

cue/load/testdata/testfetch/depnotfound.txtar

@@ -1,5 +1,5 @@
 -- out/modfetch/error --
-import failed: cannot find package "example.com": cannot fetch [email protected]: module [email protected]: 404 Not Found: name unknown: repository name not known to registry:
+import failed: cannot find package "example.com": cannot fetch [email protected]: module [email protected]: module not found:
     ./main.cue:2:8
 -- cue.mod/module.cue --
 module: "main.org@v0"

internal/mod/modrequirements/requirements_test.go

@@ -123,7 +123,7 @@ deps: "bar.com@v0": v: "v0.0.2"	// doesn't exist
 		"foo.com/bar/[email protected]",
 	), nil)
 	_, err := rs.Graph(ctx)
-	qt.Assert(t, qt.ErrorMatches(err, `[email protected]: module [email protected]: 404 Not Found: name unknown: repository name not known to registry`))
+	qt.Assert(t, qt.ErrorMatches(err, `[email protected]: module [email protected]: module not found`))
 	qt.Assert(t, qt.ErrorAs(err, new(*mvs.BuildListError[module.Version])))
 }
 

mod/modregistry/client.go

@@ -118,8 +118,7 @@ func (c *Client) GetModule(ctx context.Context, m module.Version) (*Module, erro
 	}
 	rd, err := loc.Registry.GetTag(ctx, loc.Repository, loc.Tag)
 	if err != nil {
-		// TODO should we use isNotExist here too?
-		if errors.Is(err, ociregistry.ErrManifestUnknown) {
+		if isNotExist(err) {
 			return nil, fmt.Errorf("module %v: %w", m, ErrNotFound)
 		}
 		return nil, fmt.Errorf("module %v: %w", m, err)
@@ -450,8 +449,14 @@ func isNotExist(err error) bool {
 	// without explicitly including a "denied" error code.
 	// We treat this as a "not found" error because there's
 	// nothing the user can do about it.
+	//
+	// Also, some registries return an invalid error code with a 404
+	// response (see https://cuelang.org/issue/2982), so it
+	// seems reasonable to treat that as a non-found error too.
 	if herr := ociregistry.HTTPError(nil); errors.As(err, &herr) {
-		return herr.StatusCode() == http.StatusForbidden
+		statusCode := herr.StatusCode()
+		return statusCode == http.StatusForbidden ||
+			statusCode == http.StatusNotFound
 	}
 	return false
 }

mod/modregistry/client_test.go

@@ -19,6 +19,9 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"os"
 	"path"
 	"testing"
@@ -28,6 +31,7 @@ import (
 
 	"golang.org/x/tools/txtar"
 
+	"cuelabs.dev/go/oci/ociregistry/ociclient"
 	"cuelabs.dev/go/oci/ociregistry/ocimem"
 
 	"cuelang.org/go/internal/mod/semver"
@@ -127,6 +131,43 @@ x: a.foo + something.bar
 	qt.Assert(t, qt.DeepEquals(tags, []string{"v0.5.100"}))
 }
 
+func TestNotFound(t *testing.T) {
+	// Check that we get appropriate not-found behavior when the
+	// HTTP response isn't entirely according to spec.
+	// See https://cuelang.org/issue/2982 for an example.
+	var writeResponse func(w http.ResponseWriter)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		writeResponse(w)
+	}))
+	defer srv.Close()
+	u, _ := url.Parse(srv.URL)
+	reg, err := ociclient.New(u.Host, &ociclient.Options{
+		Insecure: true,
+	})
+	qt.Assert(t, qt.IsNil(err))
+	client := NewClient(reg)
+	checkNotFound := func(writeResp func(w http.ResponseWriter)) {
+		ctx := context.Background()
+		writeResponse = writeResp
+		mv := module.MustNewVersion("foo.com/bar@v1", "v1.2.3")
+		_, err := client.GetModule(ctx, mv)
+		qt.Assert(t, qt.ErrorIs(err, ErrNotFound))
+		versions, err := client.ModuleVersions(ctx, "foo/bar")
+		qt.Assert(t, qt.IsNil(err))
+		qt.Assert(t, qt.HasLen(versions, 0))
+	}
+
+	checkNotFound(func(w http.ResponseWriter) {
+		// issue 2982
+		w.WriteHeader(http.StatusNotFound)
+		w.Write([]byte(`{"errors":[{"code":"NOT_FOUND","message":"repository playground/cue/github.com not found"}]}`))
+	})
+	checkNotFound(func(w http.ResponseWriter) {
+		w.WriteHeader(http.StatusForbidden)
+		w.Write([]byte(`some other message`))
+	})
+}
+
 func TestPutWithMetadata(t *testing.T) {
 	const testMod = `
 -- cue.mod/module.cue --