fix: cache bust on http checksum failure (#567)

denoland · Feb 6, 2025 · 72f0944 · 72f0944
1 parent cd8f47e
commit 72f0944
Show file tree

Hide file tree

Showing 4 changed files with 153 additions and 24 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -16,7 +16,7 @@ members = ["lib"]
 deno_unsync = { version = "0.4.0", default-features = false }
 thiserror = "2"
 deno_error = "0.5.3"
-sys_traits = "0.1.0"
+sys_traits = "0.1.8"
 
 [lib]
 name = "deno_graph"

diff --git a/src/graph.rs b/src/graph.rs
@@ -4765,6 +4765,22 @@ impl<'a, 'graph> Builder<'a, 'graph> {
         jsr_url_provider: &dyn JsrUrlProvider,
         module_analyzer: &dyn ModuleAnalyzer,
       ) -> Result<PendingInfoResponse, ModuleError> {
+        async fn handle_success(
+          module_analyzer: &dyn ModuleAnalyzer,
+          specifier: Url,
+          options: ParseModuleAndSourceInfoOptions<'_>,
+        ) -> Result<PendingInfoResponse, ModuleError> {
+          let is_root = options.is_root;
+          parse_module_source_and_info(module_analyzer, options)
+            .await
+            .map(|module_source_and_info| PendingInfoResponse::Module {
+              specifier: specifier.clone(),
+              module_source_and_info,
+              pending_load: None,
+              is_root,
+            })
+        }
+
         if let Some((package_nv, fut)) = maybe_version_load_fut {
           let inner = fut.await.map_err(|err| {
             ModuleError::LoadingErr(
@@ -4854,33 +4870,64 @@ impl<'a, 'graph> Builder<'a, 'graph> {
               content,
               specifier,
               maybe_headers,
-            } => parse_module_source_and_info(
-              module_analyzer,
-              ParseModuleAndSourceInfoOptions {
-                specifier: specifier.clone(),
-                maybe_headers,
-                content,
-                maybe_attribute_type: maybe_attribute_type.as_ref(),
-                maybe_referrer: maybe_range,
-                is_root,
-                is_dynamic_branch: is_dynamic,
-              },
-            )
-            .await
-            .map(|module_source_and_info| {
-              PendingInfoResponse::Module {
-                specifier: specifier.clone(),
-                module_source_and_info,
-                pending_load: None,
-                is_root,
-              }
-            }),
+            } => {
+              handle_success(
+                module_analyzer,
+                specifier.clone(),
+                ParseModuleAndSourceInfoOptions {
+                  specifier,
+                  maybe_headers,
+                  content,
+                  maybe_attribute_type: maybe_attribute_type.as_ref(),
+                  maybe_referrer: maybe_range,
+                  is_root,
+                  is_dynamic_branch: is_dynamic,
+                },
+              )
+              .await
+            }
           },
           Ok(None) => Err(ModuleError::Missing(
             load_specifier.clone(),
             maybe_range.cloned(),
           )),
           Err(LoadError::ChecksumIntegrity(err)) => {
+            if maybe_version_info.is_none() {
+              // attempt to cache bust because the remote server might have changed
+              let result = loader
+                .load(
+                  &load_specifier,
+                  LoadOptions {
+                    is_dynamic,
+                    was_dynamic_root,
+                    cache_setting: CacheSetting::Reload,
+                    maybe_checksum: maybe_checksum.clone(),
+                  },
+                )
+                .await;
+              if let Ok(Some(LoadResponse::Module {
+                content,
+                specifier,
+                maybe_headers,
+              })) = result
+              {
+                return handle_success(
+                  module_analyzer,
+                  specifier.clone(),
+                  ParseModuleAndSourceInfoOptions {
+                    specifier,
+                    maybe_headers,
+                    content,
+                    maybe_attribute_type: maybe_attribute_type.as_ref(),
+                    maybe_referrer: maybe_range,
+                    is_root,
+                    is_dynamic_branch: is_dynamic,
+                  },
+                )
+                .await;
+              }
+            }
+
             Err(ModuleError::LoadingErr(
               load_specifier.clone(),
               maybe_range.cloned(),

diff --git a/tests/integration_test.rs b/tests/integration_test.rs
@@ -13,6 +13,8 @@ use deno_graph::packages::JsrPackageInfo;
 use deno_graph::packages::JsrPackageInfoVersion;
 use deno_graph::packages::JsrPackageVersionInfo;
 use deno_graph::source::CacheSetting;
+use deno_graph::source::ChecksumIntegrityError;
+use deno_graph::source::LoadError;
 use deno_graph::source::LoadFuture;
 use deno_graph::source::LoadOptions;
 use deno_graph::source::LoadResponse;
@@ -388,6 +390,86 @@ async fn test_jsr_wasm_module() {
   }
 }
 
+#[tokio::test]
+async fn test_checksum_error_force_refresh() {
+  #[derive(Default)]
+  struct TestLoader {
+    requests: RefCell<Vec<(String, CacheSetting)>>,
+  }
+
+  impl deno_graph::source::Loader for TestLoader {
+    fn load(
+      &self,
+      specifier: &ModuleSpecifier,
+      options: LoadOptions,
+    ) -> LoadFuture {
+      self
+        .requests
+        .borrow_mut()
+        .push((specifier.to_string(), options.cache_setting));
+      let specifier = specifier.clone();
+      match specifier.as_str() {
+        "https://deno.land/mod.ts" => Box::pin(async move {
+          match options.cache_setting {
+            CacheSetting::Only => unreachable!(),
+            CacheSetting::Use => {
+              Err(LoadError::ChecksumIntegrity(ChecksumIntegrityError {
+                actual: "actual".to_string(),
+                expected: "expected".to_string(),
+              }))
+            }
+            CacheSetting::Reload => Ok(Some(LoadResponse::Module {
+              specifier: specifier.clone(),
+              maybe_headers: None,
+              content: b"import './other.js';".to_vec().into(),
+            })),
+          }
+        }),
+        "https://deno.land/other.js" => Box::pin(async move {
+          match options.cache_setting {
+            CacheSetting::Only => unreachable!(),
+            CacheSetting::Use => {
+              Err(LoadError::ChecksumIntegrity(ChecksumIntegrityError {
+                actual: "actual".to_string(),
+                expected: "expected".to_string(),
+              }))
+            }
+            CacheSetting::Reload => Ok(Some(LoadResponse::Module {
+              specifier: specifier.clone(),
+              maybe_headers: None,
+              content: b"console.log(1);".to_vec().into(),
+            })),
+          }
+        }),
+        _ => unreachable!(),
+      }
+    }
+  }
+
+  let loader = TestLoader::default();
+  let mut graph = ModuleGraph::new(GraphKind::All);
+  graph
+    .build(
+      vec![Url::parse("https://deno.land/mod.ts").unwrap()],
+      &loader,
+      Default::default(),
+    )
+    .await;
+  graph.valid().unwrap();
+  assert_eq!(
+    *loader.requests.borrow(),
+    vec![
+      ("https://deno.land/mod.ts".to_string(), CacheSetting::Use),
+      ("https://deno.land/mod.ts".to_string(), CacheSetting::Reload),
+      ("https://deno.land/other.js".to_string(), CacheSetting::Use),
+      (
+        "https://deno.land/other.js".to_string(),
+        CacheSetting::Reload
+      ),
+    ]
+  );
+}
+
 #[tokio::test]
 async fn test_dynamic_imports_with_template_arg() {
   async fn run_test(