Explain why a module hasn't been found in any registry

This makes it easier for users to discover that a module hasn't been found because its absence was recorded in the lockfile during a previous build.

Fixes bazelbuild#24803

Closes bazelbuild#24804.

PiperOrigin-RevId: 713142592
Change-Id: Id541f5710481bd947c09d8dba315d683a1666b1c

Author: fmeum

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

@@ -104,6 +104,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/util:os",

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
 import com.google.devtools.build.lib.bazel.repository.downloader.Checksum.MissingChecksumException;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.profiler.Profiler;
@@ -131,28 +132,32 @@ private String constructUrl(String base, String... segments) {
     return url.toString();
   }
 
-  /** Grabs a file from the given URL. Returns {@link Optional#empty} if the file doesn't exist. */
-  private Optional<byte[]> grabFile(
+  /** Grabs a file from the given URL. Throws {@link NotFoundException} if it doesn't exist. */
+  private byte[] grabFile(
       String url,
       ExtendedEventHandler eventHandler,
       DownloadManager downloadManager,
       boolean useChecksum)
-      throws IOException, InterruptedException {
-    var maybeContent = doGrabFile(downloadManager, url, eventHandler, useChecksum);
-    if ((knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE
-            || knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE)
-        && useChecksum) {
-      eventHandler.post(RegistryFileDownloadEvent.create(url, maybeContent));
+      throws IOException, InterruptedException, NotFoundException {
+    Optional<byte[]> maybeContent = Optional.empty();
+    try {
+      maybeContent = Optional.of(doGrabFile(downloadManager, url, eventHandler, useChecksum));
+      return maybeContent.get();
+    } finally {
+      if ((knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE
+              || knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE)
+          && useChecksum) {
+        eventHandler.post(RegistryFileDownloadEvent.create(url, maybeContent));
+      }
     }
-    return maybeContent;
   }
 
-  private Optional<byte[]> doGrabFile(
+  private byte[] doGrabFile(
       DownloadManager downloadManager,
       String rawUrl,
       ExtendedEventHandler eventHandler,
       boolean useChecksum)
-      throws IOException, InterruptedException {
+      throws IOException, InterruptedException, NotFoundException {
     Optional<Checksum> checksum;
     if (knownFileHashesMode != KnownFileHashesMode.IGNORE && useChecksum) {
       Optional<Checksum> knownChecksum = knownFileHashes.get(rawUrl);
@@ -174,7 +179,11 @@ private Optional<byte[]> doGrabFile(
           checksum = Optional.empty();
         } else {
           // Guarantee reproducibility by assuming that the file still doesn't exist.
-          return Optional.empty();
+          throw new NotFoundException(
+              String.format(
+                  "%s: previously not found (as recorded in %s, refresh with"
+                      + " --lockfile_mode=refresh)",
+                  rawUrl, LabelConstants.MODULE_LOCKFILE_NAME));
         }
       } else {
         // The file is known, download with a checksum to potentially obtain a repository cache hit
@@ -203,7 +212,7 @@ private Optional<byte[]> doGrabFile(
         && !url.getProtocol().equals("file")
         && vendorManager.isUrlVendored(url)) {
       try {
-        return Optional.of(vendorManager.readRegistryUrl(url, checksum.get()));
+        return vendorManager.readRegistryUrl(url, checksum.get());
       } catch (IOException e) {
         throw new IOException(
             String.format(
@@ -216,10 +225,9 @@ private Optional<byte[]> doGrabFile(
 
     try (SilentCloseable c =
         Profiler.instance().profile(ProfilerTask.BZLMOD, () -> "download file: " + rawUrl)) {
-      return Optional.of(
-          downloadManager.downloadAndReadOneUrlForBzlmod(url, eventHandler, clientEnv, checksum));
+      return downloadManager.downloadAndReadOneUrlForBzlmod(url, eventHandler, clientEnv, checksum);
     } catch (FileNotFoundException e) {
-      return Optional.empty();
+      throw new NotFoundException(String.format("%s: not found", rawUrl));
     } catch (IOException e) {
       // Include the URL in the exception message for easier debugging.
       throw new IOException(
@@ -228,14 +236,13 @@ private Optional<byte[]> doGrabFile(
   }
 
   @Override
-  public Optional<ModuleFile> getModuleFile(
+  public ModuleFile getModuleFile(
       ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager)
-      throws IOException, InterruptedException {
+      throws IOException, InterruptedException, NotFoundException {
     String url =
         constructUrl(getUrl(), "modules", key.name(), key.version().toString(), "MODULE.bazel");
-    Optional<byte[]> maybeContent =
-        grabFile(url, eventHandler, downloadManager, /* useChecksum= */ true);
-    return maybeContent.map(content -> ModuleFile.create(content, url));
+    byte[] content = grabFile(url, eventHandler, downloadManager, /* useChecksum= */ true);
+    return ModuleFile.create(content, url);
   }
 
   /** Represents fields available in {@code bazel_registry.json} for the registry. */
@@ -286,8 +293,12 @@ private Optional<String> grabJsonFile(
       DownloadManager downloadManager,
       boolean useChecksum)
       throws IOException, InterruptedException {
-    return grabFile(url, eventHandler, downloadManager, useChecksum)
-        .map(value -> new String(value, UTF_8));
+    try {
+      return Optional.of(
+          new String(grabFile(url, eventHandler, downloadManager, useChecksum), UTF_8));
+    } catch (NotFoundException e) {
+      return Optional.empty();
+    }
   }
 
   /**

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java

@@ -74,7 +74,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.Dict;
@@ -91,6 +90,7 @@
  */
 public class ModuleFileFunction implements SkyFunction {
 
+  // Never empty.
   public static final Precomputed<ImmutableSet<String>> REGISTRIES =
       new Precomputed<>("registries");
   public static final Precomputed<Boolean> IGNORE_DEV_DEPS =
@@ -677,14 +677,21 @@ private GetModuleFileResult getModuleFile(
     // Now go through the list of registries and use the first one that contains the requested
     // module.
     StoredEventHandler downloadEventHandler = new StoredEventHandler();
+    List<String> notFoundTrace = null;
     for (Registry registry : registryObjects) {
       try {
-        Optional<ModuleFile> maybeModuleFile =
-            registry.getModuleFile(key, downloadEventHandler, this.downloadManager);
-        if (maybeModuleFile.isEmpty()) {
+        ModuleFile originalModuleFile;
+        try {
+          originalModuleFile =
+              registry.getModuleFile(key, downloadEventHandler, this.downloadManager);
+        } catch (Registry.NotFoundException e) {
+          if (notFoundTrace == null) {
+            notFoundTrace = new ArrayList<>();
+          }
+          notFoundTrace.add(e.getMessage());
           continue;
         }
-        ModuleFile moduleFile = maybePatchModuleFile(maybeModuleFile.get(), override, env);
+        ModuleFile moduleFile = maybePatchModuleFile(originalModuleFile, override, env);
         if (moduleFile == null) {
           return null;
         }
@@ -698,7 +705,11 @@ private GetModuleFileResult getModuleFile(
       }
     }
 
-    throw errorf(Code.MODULE_NOT_FOUND, "module not found in registries: %s", key);
+    throw errorf(
+        Code.MODULE_NOT_FOUND,
+        "module %s not found in registries:\n* %s",
+        key,
+        String.join("\n* ", notFoundTrace));
   }
 
   /**

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java

@@ -28,13 +28,22 @@ public interface Registry extends NotComparableSkyValue {
   /** The URL that uniquely identifies the registry. */
   String getUrl();
 
+  /** Thrown when a file is not found in the registry. */
+  final class NotFoundException extends Exception {
+    public NotFoundException(String message) {
+      super(message);
+    }
+  }
+
   /**
    * Retrieves the contents of the module file of the module identified by {@code key} from the
-   * registry. Returns {@code Optional.empty()} when the module is not found in this registry.
+   * registry.
+   *
+   * @throws NotFoundException if the module file is not found in the registry
    */
-  Optional<ModuleFile> getModuleFile(
+  ModuleFile getModuleFile(
       ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager)
-      throws IOException, InterruptedException;
+      throws IOException, InterruptedException, NotFoundException;
 
   /**
    * Retrieves the {@link RepoSpec} object that indicates how the contents of the module identified

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java

@@ -66,12 +66,16 @@ public String getUrl() {
   }
 
   @Override
-  public Optional<ModuleFile> getModuleFile(
-      ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager) {
+  public ModuleFile getModuleFile(
+      ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager)
+      throws NotFoundException {
     String uri = String.format("%s/modules/%s/%s/MODULE.bazel", url, key.name(), key.version());
     var maybeContent = Optional.ofNullable(modules.get(key)).map(value -> value.getBytes(UTF_8));
     eventHandler.post(RegistryFileDownloadEvent.create(uri, maybeContent));
-    return maybeContent.map(content -> ModuleFile.create(content, uri));
+    if (maybeContent.isEmpty()) {
+      throw new NotFoundException("module not found: " + key);
+    }
+    return ModuleFile.create(maybeContent.get(), uri);
   }
 
   @Override

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java

@@ -102,11 +102,12 @@ public void testHttpUrl() throws Exception {
             ImmutableMap.of(),
             Optional.empty());
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
-        .hasValue(
+        .isEqualTo(
             ModuleFile.create(
                 "lol".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
-    assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
-        .isEmpty();
+    assertThrows(
+        Registry.NotFoundException.class,
+        () -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
   }
 
   @Test
@@ -137,11 +138,12 @@ public void testHttpUrlWithNetrcCreds() throws Exception {
 
     downloadManager.setNetrcCreds(new NetrcCredentials(netrc));
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
-        .hasValue(
+        .isEqualTo(
             ModuleFile.create(
                 "lol".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
-    assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
-        .isEmpty();
+    assertThrows(
+        Registry.NotFoundException.class,
+        () -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
   }
 
   @Test
@@ -160,9 +162,10 @@ public void testFileUrl() throws Exception {
             ImmutableMap.of(),
             Optional.empty());
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
-        .hasValue(ModuleFile.create("lol".getBytes(UTF_8), file.toURI().toString()));
-    assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
-        .isEmpty();
+        .isEqualTo(ModuleFile.create("lol".getBytes(UTF_8), file.toURI().toString()));
+    assertThrows(
+        Registry.NotFoundException.class,
+        () -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
   }
 
   @Test
@@ -444,15 +447,20 @@ public void testGetModuleFileChecksums() throws Exception {
             ImmutableMap.of(),
             Optional.empty());
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
-        .hasValue(
+        .isEqualTo(
             ModuleFile.create(
                 "old".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
     assertThat(registry.getModuleFile(createModuleKey("foo", "2.0"), reporter, downloadManager))
-        .hasValue(
+        .isEqualTo(
             ModuleFile.create(
                 "new".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/2.0/MODULE.bazel"));
-    assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
-        .isEmpty();
+    var e =
+        assertThrows(
+            Registry.NotFoundException.class,
+            () -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo(server.getUrl() + "/myreg/modules/bar/1.0/MODULE.bazel: not found");
 
     var recordedChecksums = eventRecorder.getRecordedHashes();
     assertThat(
@@ -467,7 +475,7 @@ public void testGetModuleFileChecksums() throws Exception {
             Optional.empty())
         .inOrder();
 
-    registry =
+    Registry registry2 =
         registryFactory.createRegistry(
             server.getUrl() + "/myreg",
             LockfileMode.UPDATE,
@@ -479,16 +487,25 @@ public void testGetModuleFileChecksums() throws Exception {
     server.unserve("/myreg/modules/foo/1.0/MODULE.bazel");
     server.unserve("/myreg/modules/foo/2.0/MODULE.bazel");
     server.serve("/myreg/modules/bar/1.0/MODULE.bazel", "no longer 404");
-    assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
-        .hasValue(
+    assertThat(registry2.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
+        .isEqualTo(
             ModuleFile.create(
                 "old".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
-    assertThat(registry.getModuleFile(createModuleKey("foo", "2.0"), reporter, downloadManager))
-        .hasValue(
+    assertThat(registry2.getModuleFile(createModuleKey("foo", "2.0"), reporter, downloadManager))
+        .isEqualTo(
             ModuleFile.create(
                 "new".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/2.0/MODULE.bazel"));
-    assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
-        .isEmpty();
+    e =
+        assertThrows(
+            Registry.NotFoundException.class,
+            () ->
+                registry2.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo(
+            server.getUrl()
+                + "/myreg/modules/bar/1.0/MODULE.bazel: previously not found (as recorded in"
+                + " MODULE.bazel.lock, refresh with --lockfile_mode=refresh)");
   }
 
   @Test

src/test/py/bazel/bzlmod/bazel_overrides_test.py

@@ -495,8 +495,8 @@ def testCmdRelativeModuleOverride(self):
         allow_failure=True,
     )
     self.assertIn(
-        'ERROR: Error computing the main repository mapping: module not found'
-        ' in registries: [email protected]',
+        'ERROR: Error computing the main repository mapping: module [email protected] not'
+        ' found in registries:',
         stderr,
     )