fix(install): handle when bin entry info isn't present in package.json but is in registry (#28822)

Apparently things like the `bin` field can appear in the version info
from the registry, but not the package's `package.json`. I'm still not
sure how you actually achieve this, but it's the case for
`esbuild-wasm`. This fixes the following panic:

```
❯ deno i --node-modules-dir npm:esbuild-wasm
Add npm:esbuild-wasm@0.25.2
Initialize ⣯ [00:00]
 - esbuild-wasm@0.25.2



============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos aarch64
Version: 2.2.8+58c6c0b
Args: ["deno", "i", "--node-modules-dir", "npm:esbuild-wasm"]

View stack trace at:
https://panic.deno.com/v2.2.8+58c6c0bc9c1b4ee08645be936ff9268f17028f0f/aarch64-apple-darwin/g4h6Jo393pB4k4kqBo-3kqBg6klqBogtyLg13yLw_t0Lw549Hgj8-Hgw__H428-F4yv_HgjkpKww7gIon4gIw54rKwi5MorzMw5y7G42g7Iw---I40s-I4vu4Jw2rEw8z7Dwnr6J4tp7Bo_vvK

thread 'main' panicked at cli/npm/installer/common/bin_entries.rs:108:30:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
This commit is contained in:
Nathan Whitaker 2025-04-10 09:36:26 -07:00 committed by GitHub
parent ac5c6018a8
commit ce5b9da11b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 226 additions and 38 deletions

View file

@ -437,7 +437,10 @@ async fn resolve_custom_commands_from_packages<
extra.clone()
} else {
let Ok(extra) = provider
.get_package_extra_info(&package.id.nv, package.is_deprecated)
.get_package_extra_info(
&package.id.nv,
super::ExpectedExtraInfo::from_package(package),
)
.await
else {
continue;

View file

@ -8,6 +8,7 @@ use deno_core::parking_lot::RwLock;
use deno_error::JsErrorBox;
use deno_npm::registry::NpmRegistryApi;
use deno_npm::NpmPackageExtraInfo;
use deno_npm::NpmResolutionPackage;
use deno_semver::package::PackageNv;
use super::PackageCaching;
@ -29,7 +30,7 @@ pub trait NpmPackageExtraInfoProvider: std::fmt::Debug + Send + Sync {
async fn get_package_extra_info(
&self,
package_id: &PackageNv,
is_deprecated: bool,
expected: ExpectedExtraInfo,
) -> Result<deno_npm::NpmPackageExtraInfo, JsErrorBox>;
}
@ -39,11 +40,11 @@ impl<T: NpmPackageExtraInfoProvider + ?Sized> NpmPackageExtraInfoProvider
async fn get_package_extra_info(
&self,
package_id: &PackageNv,
is_deprecated: bool,
expected: ExpectedExtraInfo,
) -> Result<deno_npm::NpmPackageExtraInfo, JsErrorBox> {
self
.as_ref()
.get_package_extra_info(package_id, is_deprecated)
.get_package_extra_info(package_id, expected)
.await
}
}
@ -76,46 +77,96 @@ impl ExtraInfoProvider {
}
}
#[derive(Debug, Clone, Copy, Default)]
pub struct ExpectedExtraInfo {
pub deprecated: bool,
pub bin: bool,
pub scripts: bool,
}
impl ExpectedExtraInfo {
pub fn from_package(package: &NpmResolutionPackage) -> Self {
Self {
deprecated: package.is_deprecated,
bin: package.has_bin,
scripts: package.has_scripts,
}
}
}
impl ExtraInfoProvider {
async fn fetch_from_registry(
&self,
package_nv: &PackageNv,
) -> Result<NpmPackageExtraInfo, JsErrorBox> {
let package_info = self
.npm_registry_info_provider
.package_info(&package_nv.name)
.await
.map_err(JsErrorBox::from_err)?;
let patched_packages = HashMap::new();
let version_info = package_info
.version_info(package_nv, &patched_packages)
.map_err(JsErrorBox::from_err)?;
Ok(NpmPackageExtraInfo {
deprecated: version_info.deprecated.clone(),
bin: version_info.bin.clone(),
scripts: version_info.scripts.clone(),
})
}
async fn fetch_from_package_json(
&self,
package_nv: &PackageNv,
) -> Result<NpmPackageExtraInfo, JsErrorBox> {
let folder_path = self.npm_cache.package_folder_for_nv(package_nv);
let package_json_path = folder_path.join("package.json");
let extra_info: NpmPackageExtraInfo =
tokio::task::spawn_blocking(move || {
let package_json = std::fs::read_to_string(&package_json_path)
.map_err(JsErrorBox::from_err)?;
let extra_info: NpmPackageExtraInfo =
deno_core::serde_json::from_str(&package_json)
.map_err(JsErrorBox::from_err)?;
Ok::<_, JsErrorBox>(extra_info)
})
.await
.map_err(JsErrorBox::from_err)??;
Ok(extra_info)
}
}
impl super::common::NpmPackageExtraInfoProvider for ExtraInfoProvider {
async fn get_package_extra_info(
&self,
package_nv: &PackageNv,
deprecated: bool,
expected: ExpectedExtraInfo,
) -> Result<NpmPackageExtraInfo, JsErrorBox> {
if let Some(extra_info) = self.cache.read().get(package_nv) {
return Ok(extra_info.clone());
}
if deprecated {
let extra_info = if expected.deprecated {
// we need the registry version info to get the deprecated string, since it's not in the
// package's package.json
let package_info = self
.npm_registry_info_provider
.package_info(&package_nv.name)
.await
.map_err(JsErrorBox::from_err)?;
let patched_packages = HashMap::new();
let version_info = package_info
.version_info(package_nv, &patched_packages)
.map_err(JsErrorBox::from_err)?;
Ok(NpmPackageExtraInfo {
deprecated: version_info.deprecated.clone(),
bin: version_info.bin.clone(),
scripts: version_info.scripts.clone(),
})
self.fetch_from_registry(package_nv).await?
} else {
let folder_path = self.npm_cache.package_folder_for_nv(package_nv);
let package_json_path = folder_path.join("package.json");
let package_json = std::fs::read_to_string(&package_json_path)
.map_err(JsErrorBox::from_err)?;
let extra_info: NpmPackageExtraInfo =
deno_core::serde_json::from_str(&package_json)
.map_err(JsErrorBox::from_err)?;
self
.cache
.write()
.insert(package_nv.clone(), extra_info.clone());
Ok(extra_info)
}
let extra_info = self.fetch_from_package_json(package_nv).await?;
// some packages have bin in registry but not in package.json (e.g. esbuild-wasm)
// still not sure how that happens
if (expected.bin && extra_info.bin.is_none())
|| (expected.scripts && extra_info.scripts.is_empty())
{
self.fetch_from_registry(package_nv).await?
} else {
extra_info
}
};
self
.cache
.write()
.insert(package_nv.clone(), extra_info.clone());
Ok(extra_info)
}
}

View file

@ -414,12 +414,16 @@ async fn sync_resolution_with_fs(
Ok::<_, SyncResolutionWithFsError>(())
}
});
let extra_fut = if package.has_bin
let extra_fut = if (package.has_bin
|| package.has_scripts
|| package.is_deprecated && package.extra.is_none()
|| package.is_deprecated)
&& package.extra.is_none()
{
extra_info_provider
.get_package_extra_info(&package.id.nv, package.is_deprecated)
.get_package_extra_info(
&package.id.nv,
super::common::ExpectedExtraInfo::from_package(package),
)
.boxed_local()
} else {
std::future::ready(Ok(package.extra.clone().unwrap_or_default()))
@ -465,7 +469,10 @@ async fn sync_resolution_with_fs(
cache_futures.push(
async move {
let extra = extra_info_provider
.get_package_extra_info(&package.id.nv, package.is_deprecated)
.get_package_extra_info(
&package.id.nv,
super::common::ExpectedExtraInfo::from_package(package),
)
.await
.map_err(JsErrorBox::from_err)?;