Skip to content

Commit

Permalink
fix(wasix): Skip unavailable versions during package resolution
Browse files Browse the repository at this point in the history
The api type definitions were invalid because some fields are actually
optional (manifest, pirita_sha256_hash, pirita_download_url).

If those were null, the version resolution would fail, even if valid
packages exist.

Fixed by making the fields optional, and just ignoring versions that
don't have the required data.
  • Loading branch information
theduke committed May 19, 2023
1 parent 4e86fc5 commit a95cbdc
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 17 deletions.
8 changes: 4 additions & 4 deletions lib/wasi/src/os/console/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,12 @@ impl Console {

let binary = match resolved_package {
Ok(pkg) => pkg,
Err(e) => {
Err(err) => {
let mut stderr = self.stderr.clone();
tasks.block_on(async {
let mut buffer = Vec::new();
writeln!(buffer, "Error: {e}").ok();
let mut source = e.source();
writeln!(buffer, "Error: {err}").ok();
let mut source = err.source();
while let Some(s) = source {
writeln!(buffer, " Caused by: {s}").ok();
source = s.source();
Expand All @@ -228,7 +228,7 @@ impl Console {
.await
.ok();
});
tracing::debug!("failed to get webc dependency - {}", webc);
tracing::debug!(error = &*err, "failed to get webc dependency - {}", webc);
return Err(VirtualBusError::NotFound);
}
};
Expand Down
10 changes: 10 additions & 0 deletions lib/wasi/src/runtime/package_loader/builtin_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,14 @@ impl BuiltinPackageLoader {
Ok(None)
}

#[tracing::instrument(
level="debug",
skip_all,
fields(url=%dist.webc, hash=%dist.webc_sha256),
)]
async fn download(&self, dist: &DistributionInfo) -> Result<Bytes, Error> {
tracing::trace!("retrieving webc");

if dist.webc.scheme() == "file" {
// Note: The Url::to_file_path() method is platform-specific
#[cfg(any(unix, windows, target_os = "redox", target_os = "wasi"))]
Expand All @@ -96,6 +103,8 @@ impl BuiltinPackageLoader {
}
}

tracing::debug!("downloading webc");

let request = HttpRequest {
url: dist.webc.to_string(),
method: "GET".to_string(),
Expand All @@ -116,6 +125,7 @@ impl BuiltinPackageLoader {
} = self.client.request(request).await?;

if !ok {
tracing::debug!(%status, %status_text, "failed to download webc");
anyhow::bail!("{status} {status_text}");
}

Expand Down
44 changes: 31 additions & 13 deletions lib/wasi/src/runtime/resolver/wapm_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ impl WapmSource {

#[async_trait::async_trait]
impl Source for WapmSource {
#[tracing::instrument(
level = "debug",
skip_all,
fields(%package),
)]
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<Summary>, Error> {
let (full_name, version_constraint) = match package {
PackageSpecifier::Registry { full_name, version } => (full_name, version),
Expand Down Expand Up @@ -86,8 +91,16 @@ impl Source for WapmSource {
for pkg_version in response.data.get_package.versions {
let version = Version::parse(&pkg_version.version)?;
if version_constraint.matches(&version) {
let summary = decode_summary(pkg_version)?;
summaries.push(summary);
match decode_summary(pkg_version) {
Ok(summary) => {
summaries.push(summary);
}
Err(err) => {
// Do not abort on errors, because the API might return
// some invalid packages.
tracing::warn!(error = &*err, "unable to decode package summary");
}
}
}
}

Expand All @@ -97,7 +110,7 @@ impl Source for WapmSource {

fn decode_summary(pkg_version: WapmWebQueryGetPackageVersion) -> Result<Summary, Error> {
let WapmWebQueryGetPackageVersion {
manifest,
manifest: manifest_opt,
distribution:
WapmWebQueryGetPackageVersionDistribution {
pirita_download_url,
Expand All @@ -106,19 +119,24 @@ fn decode_summary(pkg_version: WapmWebQueryGetPackageVersion) -> Result<Summary,
..
} = pkg_version;

let manifest: Manifest = serde_json::from_slice(manifest.as_bytes())
.context("Unable to deserialize the manifest")?;
let manifest_str = manifest_opt.context("package version has no manifest")?;

let manifest: Manifest =
serde_json::from_str(&manifest_str).context("Unable to deserialize the manifest")?;

let hash = pirita_sha256_hash.context("package version has no webc hash")?;

let mut webc_sha256 = [0_u8; 32];
hex::decode_to_slice(&pirita_sha256_hash, &mut webc_sha256)?;
hex::decode_to_slice(&hash, &mut webc_sha256)?;
let webc_sha256 = WebcHash::from_bytes(webc_sha256);

let webc: url::Url = pirita_download_url
.context("package version has no webc download url")?
.parse()?;

Ok(Summary {
pkg: PackageInfo::from_manifest(&manifest)?,
dist: DistributionInfo {
webc: pirita_download_url.parse()?,
webc_sha256,
},
dist: DistributionInfo { webc, webc_sha256 },
})
}

Expand Down Expand Up @@ -158,16 +176,16 @@ pub struct WapmWebQueryGetPackageVersion {
pub version: String,
/// A JSON string containing a [`Manifest`] definition.
#[serde(rename = "piritaManifest")]
pub manifest: String,
pub manifest: Option<String>,
pub distribution: WapmWebQueryGetPackageVersionDistribution,
}

#[derive(Debug, serde::Serialize, serde::Deserialize, Clone)]
pub struct WapmWebQueryGetPackageVersionDistribution {
#[serde(rename = "piritaDownloadUrl")]
pub pirita_download_url: String,
pub pirita_download_url: Option<String>,
#[serde(rename = "piritaSha256Hash")]
pub pirita_sha256_hash: String,
pub pirita_sha256_hash: Option<String>,
}

#[cfg(test)]
Expand Down

0 comments on commit a95cbdc

Please sign in to comment.