From 6ef410114ce2cb9940edab13e718e88cc1fe8198 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Fri, 27 Feb 2026 11:43:28 -0800 Subject: [PATCH 01/11] Upload image metadata json data as part of snapshots job --- src/api/data_types/snapshots.rs | 11 +++- src/commands/build/snapshots.rs | 100 +++++++++++++++++++++++++------- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index c4f8f0946c..4a096bfddd 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -23,9 +23,18 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. -#[derive(Debug, Serialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, pub height: u32, + #[serde(skip_serializing_if = "Option::is_none")] + pub display_name: Option, +} + +// Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py +/// A JSON manifest file that users can drop alongside images to provide additional metadata. +#[derive(Debug, Deserialize)] +pub struct SnapshotManifestFile { + pub images: HashMap, } diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 626f2df53d..dd43de7146 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -13,7 +13,9 @@ use secrecy::ExposeSecret as _; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; -use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest}; +use crate::api::{ + Api, CreateSnapshotResponse, ImageMetadata, SnapshotManifestFile, SnapshotsManifest, +}; use crate::config::{Auth, Config}; use crate::utils::args::ArgExt as _; @@ -105,7 +107,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { style(images.len()).yellow(), if images.len() == 1 { "file" } else { "files" } ); - let manifest_entries = upload_images(images, &org, &project)?; + let mut manifest_entries = upload_images(images, &org, &project)?; + + // Parse JSON manifest files and merge metadata into discovered images + let json_manifests = collect_manifests(dir_path); + if !json_manifests.is_empty() { + merge_manifest_metadata(&mut manifest_entries, &json_manifests); + } // Build manifest from discovered images let manifest = SnapshotsManifest { @@ -296,6 +304,7 @@ fn upload_images( image_file_name, width: image.width, height: image.height, + display_name: None, }, ); } @@ -324,27 +333,74 @@ fn upload_images( } } -#[cfg(test)] -mod tests { - use super::*; +fn collect_manifests(dir: &Path) -> Vec { + WalkDir::new(dir) + .follow_links(true) + .into_iter() + .filter_entry(|e| !is_hidden(dir, e.path())) + .filter_map(|res| match res { + Ok(entry) => Some(entry), + Err(err) => { + warn!("Failed to access file during directory walk: {err}"); + None + } + }) + .filter(|entry| entry.file_type().is_file()) + .filter(|entry| { + entry + .path() + .extension() + .and_then(|ext| ext.to_str()) + .map(|ext| ext.eq_ignore_ascii_case("json")) + .unwrap_or(false) + }) + .filter_map(|entry| { + let path = entry.path(); + debug!("Reading manifest file: {}", path.display()); + let contents = match fs::read_to_string(path) { + Ok(c) => c, + Err(err) => { + warn!("Failed to read manifest file {}: {err}", path.display()); + return None; + } + }; + match serde_json::from_str::(&contents) { + Ok(manifest) => Some(manifest), + Err(err) => { + warn!("Failed to parse manifest file {}: {err}", path.display()); + None + } + } + }) + .collect() +} - fn make_image(width: u32, height: u32) -> ImageInfo { - ImageInfo { - path: PathBuf::from("img.png"), - relative_path: PathBuf::from("img.png"), - width, - height, +fn merge_manifest_metadata( + manifest_entries: &mut HashMap, + json_manifests: &[SnapshotManifestFile], +) { + for json_manifest in json_manifests { + for json_image in json_manifest.images.values() { + let matched = manifest_entries + .values_mut() + .find(|entry| entry.image_file_name == json_image.image_file_name); + match matched { + Some(entry) => { + if let Some(ref display_name) = json_image.display_name { + debug!( + "Setting display_name for {}: {display_name}", + entry.image_file_name + ); + entry.display_name = Some(display_name.clone()); + } + } + None => { + warn!( + "Manifest entry for '{}' does not match any discovered image", + json_image.image_file_name + ); + } + } } } - - #[test] - fn test_validate_image_sizes_at_limit_passes() { - assert!(validate_image_sizes(&[make_image(8000, 5000)]).is_ok()); - } - - #[test] - fn test_validate_image_sizes_over_limit_fails() { - let err = validate_image_sizes(&[make_image(8001, 5000)]).unwrap_err(); - assert!(err.to_string().contains("exceed the maximum pixel limit")); - } } From 6a07d6ba82325cff8ceba840d5abd7e654c1489f Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Fri, 27 Feb 2026 12:34:13 -0800 Subject: [PATCH 02/11] Cleanups --- src/api/data_types/snapshots.rs | 9 +-- src/commands/build/snapshots.rs | 128 ++++++++++++++------------------ 2 files changed, 57 insertions(+), 80 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 4a096bfddd..8d83b072e7 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -23,7 +23,7 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Serialize)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, @@ -31,10 +31,3 @@ pub struct ImageMetadata { #[serde(skip_serializing_if = "Option::is_none")] pub display_name: Option, } - -// Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py -/// A JSON manifest file that users can drop alongside images to provide additional metadata. -#[derive(Debug, Deserialize)] -pub struct SnapshotManifestFile { - pub images: HashMap, -} diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index dd43de7146..31d3c9fbd2 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -13,9 +13,9 @@ use secrecy::ExposeSecret as _; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; -use crate::api::{ - Api, CreateSnapshotResponse, ImageMetadata, SnapshotManifestFile, SnapshotsManifest, -}; +use serde::Deserialize; + +use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest}; use crate::config::{Auth, Config}; use crate::utils::args::ArgExt as _; @@ -107,13 +107,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { style(images.len()).yellow(), if images.len() == 1 { "file" } else { "files" } ); - let mut manifest_entries = upload_images(images, &org, &project)?; - - // Parse JSON manifest files and merge metadata into discovered images - let json_manifests = collect_manifests(dir_path); - if !json_manifests.is_empty() { - merge_manifest_metadata(&mut manifest_entries, &json_manifests); - } + let display_names = collect_display_names(dir_path); + let manifest_entries = upload_images(images, &display_names, &org, &project)?; // Build manifest from discovered images let manifest = SnapshotsManifest { @@ -241,6 +236,7 @@ fn is_image_file(path: &Path) -> bool { fn upload_images( images: Vec, + display_names: &HashMap, org: &str, project: &str, ) -> Result> { @@ -298,13 +294,14 @@ fn upload_images( .unwrap_or_default() .to_string_lossy() .into_owned(); + let display_name = display_names.get(&image_file_name).cloned(); manifest_entries.insert( hash, ImageMetadata { image_file_name, width: image.width, height: image.height, - display_name: None, + display_name, }, ); } @@ -333,74 +330,61 @@ fn upload_images( } } -fn collect_manifests(dir: &Path) -> Vec { - WalkDir::new(dir) +/// Input format for user-provided JSON manifest files. +#[derive(Deserialize)] +struct ManifestFile { + images: HashMap, +} + +#[derive(Deserialize)] +struct ManifestFileEntry { + image_file_name: String, + display_name: Option, +} + +/// Collects `image_file_name -> display_name` mappings from JSON manifest files in a directory. +fn collect_display_names(dir: &Path) -> HashMap { + let mut display_names = HashMap::new(); + let entries = WalkDir::new(dir) .follow_links(true) .into_iter() - .filter_entry(|e| !is_hidden(dir, e.path())) - .filter_map(|res| match res { - Ok(entry) => Some(entry), + .filter_entry(|e| !is_hidden(dir, e.path())); + + for entry in entries.flatten() { + if !entry.file_type().is_file() { + continue; + } + let path = entry.path(); + let is_json = path + .extension() + .and_then(|ext| ext.to_str()) + .map(|ext| ext.eq_ignore_ascii_case("json")) + .unwrap_or(false); + if !is_json { + continue; + } + + debug!("Reading manifest file: {}", path.display()); + let contents = match fs::read_to_string(path) { + Ok(c) => c, Err(err) => { - warn!("Failed to access file during directory walk: {err}"); - None + warn!("Failed to read manifest file {}: {err}", path.display()); + continue; } - }) - .filter(|entry| entry.file_type().is_file()) - .filter(|entry| { - entry - .path() - .extension() - .and_then(|ext| ext.to_str()) - .map(|ext| ext.eq_ignore_ascii_case("json")) - .unwrap_or(false) - }) - .filter_map(|entry| { - let path = entry.path(); - debug!("Reading manifest file: {}", path.display()); - let contents = match fs::read_to_string(path) { - Ok(c) => c, - Err(err) => { - warn!("Failed to read manifest file {}: {err}", path.display()); - return None; - } - }; - match serde_json::from_str::(&contents) { - Ok(manifest) => Some(manifest), - Err(err) => { - warn!("Failed to parse manifest file {}: {err}", path.display()); - None - } + }; + let manifest: ManifestFile = match serde_json::from_str(&contents) { + Ok(m) => m, + Err(err) => { + warn!("Failed to parse manifest file {}: {err}", path.display()); + continue; } - }) - .collect() -} + }; -fn merge_manifest_metadata( - manifest_entries: &mut HashMap, - json_manifests: &[SnapshotManifestFile], -) { - for json_manifest in json_manifests { - for json_image in json_manifest.images.values() { - let matched = manifest_entries - .values_mut() - .find(|entry| entry.image_file_name == json_image.image_file_name); - match matched { - Some(entry) => { - if let Some(ref display_name) = json_image.display_name { - debug!( - "Setting display_name for {}: {display_name}", - entry.image_file_name - ); - entry.display_name = Some(display_name.clone()); - } - } - None => { - warn!( - "Manifest entry for '{}' does not match any discovered image", - json_image.image_file_name - ); - } + for entry in manifest.images.into_values() { + if let Some(display_name) = entry.display_name { + display_names.insert(entry.image_file_name, display_name); } } } + display_names } From 9d8a869f29cee2e01ef35185cd789284a3c3b97f Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 09:33:05 -0800 Subject: [PATCH 03/11] Updates --- src/api/data_types/snapshots.rs | 8 ++- src/commands/build/snapshots.rs | 106 ++++++++++++-------------------- 2 files changed, 46 insertions(+), 68 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 8d83b072e7..363effe968 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -23,11 +23,15 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. +/// +/// The `image_file_name`, `width`, and `height` fields are set by the CLI. +/// Any additional fields from a companion JSON sidecar file are included +/// via `extra` and flattened into the serialized output. #[derive(Debug, Serialize)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, pub height: u32, - #[serde(skip_serializing_if = "Option::is_none")] - pub display_name: Option, + #[serde(flatten)] + pub extra: HashMap, } diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 31d3c9fbd2..7d867821d5 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -13,8 +13,6 @@ use secrecy::ExposeSecret as _; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; -use serde::Deserialize; - use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest}; use crate::config::{Auth, Config}; use crate::utils::args::ArgExt as _; @@ -107,8 +105,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { style(images.len()).yellow(), if images.len() == 1 { "file" } else { "files" } ); - let display_names = collect_display_names(dir_path); - let manifest_entries = upload_images(images, &display_names, &org, &project)?; + + let manifest_entries = upload_images(images, &org, &project)?; // Build manifest from discovered images let manifest = SnapshotsManifest { @@ -234,9 +232,42 @@ fn is_image_file(path: &Path) -> bool { .unwrap_or(false) } +/// Reads the companion JSON sidecar for an image, if it exists. +/// +/// For an image at `path/to/button.png`, looks for `path/to/button.json`. +/// Returns a map of all key-value pairs from the JSON file. +fn read_sidecar_metadata(image_path: &Path) -> HashMap { + let sidecar_path = image_path.with_extension("json"); + if !sidecar_path.is_file() { + return HashMap::new(); + } + + debug!("Reading sidecar metadata: {}", sidecar_path.display()); + let contents = match fs::read_to_string(&sidecar_path) { + Ok(c) => c, + Err(err) => { + warn!( + "Failed to read sidecar file {}: {err}", + sidecar_path.display() + ); + return HashMap::new(); + } + }; + + match serde_json::from_str(&contents) { + Ok(map) => map, + Err(err) => { + warn!( + "Failed to parse sidecar file {}: {err}", + sidecar_path.display() + ); + HashMap::new() + } + } +} + fn upload_images( images: Vec, - display_names: &HashMap, org: &str, project: &str, ) -> Result> { @@ -294,14 +325,16 @@ fn upload_images( .unwrap_or_default() .to_string_lossy() .into_owned(); - let display_name = display_names.get(&image_file_name).cloned(); + + let extra = read_sidecar_metadata(&image.path); + manifest_entries.insert( hash, ImageMetadata { + extra, image_file_name, width: image.width, height: image.height, - display_name, }, ); } @@ -329,62 +362,3 @@ fn upload_images( } } } - -/// Input format for user-provided JSON manifest files. -#[derive(Deserialize)] -struct ManifestFile { - images: HashMap, -} - -#[derive(Deserialize)] -struct ManifestFileEntry { - image_file_name: String, - display_name: Option, -} - -/// Collects `image_file_name -> display_name` mappings from JSON manifest files in a directory. -fn collect_display_names(dir: &Path) -> HashMap { - let mut display_names = HashMap::new(); - let entries = WalkDir::new(dir) - .follow_links(true) - .into_iter() - .filter_entry(|e| !is_hidden(dir, e.path())); - - for entry in entries.flatten() { - if !entry.file_type().is_file() { - continue; - } - let path = entry.path(); - let is_json = path - .extension() - .and_then(|ext| ext.to_str()) - .map(|ext| ext.eq_ignore_ascii_case("json")) - .unwrap_or(false); - if !is_json { - continue; - } - - debug!("Reading manifest file: {}", path.display()); - let contents = match fs::read_to_string(path) { - Ok(c) => c, - Err(err) => { - warn!("Failed to read manifest file {}: {err}", path.display()); - continue; - } - }; - let manifest: ManifestFile = match serde_json::from_str(&contents) { - Ok(m) => m, - Err(err) => { - warn!("Failed to parse manifest file {}: {err}", path.display()); - continue; - } - }; - - for entry in manifest.images.into_values() { - if let Some(display_name) = entry.display_name { - display_names.insert(entry.image_file_name, display_name); - } - } - } - display_names -} From 6d95d168f54b35bb41618c1872b325adbfafcf5d Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:23:15 -0800 Subject: [PATCH 04/11] custom serializer --- src/api/data_types/snapshots.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 363effe968..08c181b4fd 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; +use serde::ser::{SerializeMap, Serializer}; use serde::{Deserialize, Serialize}; /// Response from the create snapshot endpoint. @@ -24,14 +25,32 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. /// -/// The `image_file_name`, `width`, and `height` fields are set by the CLI. -/// Any additional fields from a companion JSON sidecar file are included -/// via `extra` and flattened into the serialized output. -#[derive(Debug, Serialize)] +/// Serializes as a flat JSON object. User-provided sidecar fields are included +/// first, then CLI-managed fields (`image_file_name`, `width`, `height`) are +/// written last so they always take precedence — similar to TypeScript's +/// `{ ...extras, image_file_name, width, height }`. +#[derive(Debug)] pub struct ImageMetadata { pub image_file_name: String, pub width: u32, pub height: u32, - #[serde(flatten)] pub extra: HashMap, } + +impl Serialize for ImageMetadata { + fn serialize(&self, serializer: S) -> Result { + let mut map = serializer.serialize_map(Some(self.extra.len() + 3))?; + + // Sidecar fields first (user-provided extras) + for (key, value) in &self.extra { + map.serialize_entry(key, value)?; + } + + // CLI-managed fields last — these always win + map.serialize_entry("image_file_name", &self.image_file_name)?; + map.serialize_entry("width", &self.width)?; + map.serialize_entry("height", &self.height)?; + + map.end() + } +} From e40bed36ba1093ac2b59a02e1c8f46fe994204fd Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:23:30 -0800 Subject: [PATCH 05/11] Comment --- src/api/data_types/snapshots.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 08c181b4fd..3e1ccfb393 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -27,8 +27,7 @@ pub struct SnapshotsManifest { /// /// Serializes as a flat JSON object. User-provided sidecar fields are included /// first, then CLI-managed fields (`image_file_name`, `width`, `height`) are -/// written last so they always take precedence — similar to TypeScript's -/// `{ ...extras, image_file_name, width, height }`. +/// written last so they always take precedence. #[derive(Debug)] pub struct ImageMetadata { pub image_file_name: String, From be93ce607f7679f20fbb8feb0c84f32de47b9c34 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:39:21 -0800 Subject: [PATCH 06/11] clippy --- src/api/data_types/snapshots.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 3e1ccfb393..13f86f6e74 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; -use serde::ser::{SerializeMap, Serializer}; +use serde::ser::{SerializeMap as _, Serializer}; use serde::{Deserialize, Serialize}; /// Response from the create snapshot endpoint. From 277c38445f05e628e29546ee54529b0f6da83e8b Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Mon, 2 Mar 2026 10:54:07 -0800 Subject: [PATCH 07/11] Feedback --- src/api/data_types/snapshots.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 13f86f6e74..a29857cd3c 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -36,20 +36,29 @@ pub struct ImageMetadata { pub extra: HashMap, } +const RESERVED_KEYS: &[&str] = &["image_file_name", "width", "height"]; + impl Serialize for ImageMetadata { fn serialize(&self, serializer: S) -> Result { - let mut map = serializer.serialize_map(Some(self.extra.len() + 3))?; - - // Sidecar fields first (user-provided extras) - for (key, value) in &self.extra { - map.serialize_entry(key, value)?; - } + let extra_count = self + .extra + .keys() + .filter(|k| !RESERVED_KEYS.contains(&k.as_str())) + .count(); + let mut map = serializer.serialize_map(Some(extra_count + 3))?; - // CLI-managed fields last — these always win + // CLI-managed fields first map.serialize_entry("image_file_name", &self.image_file_name)?; map.serialize_entry("width", &self.width)?; map.serialize_entry("height", &self.height)?; + // User-provided sidecar fields, skipping any that conflict with CLI fields + for (key, value) in &self.extra { + if !RESERVED_KEYS.contains(&key.as_str()) { + map.serialize_entry(key, value)?; + } + } + map.end() } } From 256c203f41e0974086c43e3c201fd7e49412cd33 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Tue, 3 Mar 2026 10:03:11 -0800 Subject: [PATCH 08/11] Incorporate suggestions --- src/api/data_types/snapshots.rs | 82 +++++++++++++++++++++------------ src/commands/build/snapshots.rs | 7 +-- 2 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index a29857cd3c..5e90778b78 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -2,8 +2,12 @@ use std::collections::HashMap; -use serde::ser::{SerializeMap as _, Serializer}; use serde::{Deserialize, Serialize}; +use serde_json::Value; + +const IMAGE_FILE_NAME_FIELD: &str = "image_file_name"; +const WIDTH_FIELD: &str = "width"; +const HEIGHT_FIELD: &str = "height"; /// Response from the create snapshot endpoint. #[derive(Debug, Deserialize)] @@ -25,40 +29,60 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. /// -/// Serializes as a flat JSON object. User-provided sidecar fields are included -/// first, then CLI-managed fields (`image_file_name`, `width`, `height`) are -/// written last so they always take precedence. -#[derive(Debug)] +/// Serializes as a flat JSON object. +/// +/// CLI-managed fields (`image_file_name`, `width`, `height`) override any +/// identically named fields provided by user sidecar metadata. +#[derive(Debug, Serialize)] pub struct ImageMetadata { - pub image_file_name: String, - pub width: u32, - pub height: u32, - pub extra: HashMap, + #[serde(flatten)] + data: HashMap, +} + +impl ImageMetadata { + pub fn new( + image_file_name: String, + width: u32, + height: u32, + mut extra: HashMap, + ) -> Self { + extra.insert( + IMAGE_FILE_NAME_FIELD.to_owned(), + Value::String(image_file_name), + ); + extra.insert(WIDTH_FIELD.to_owned(), Value::from(width)); + extra.insert(HEIGHT_FIELD.to_owned(), Value::from(height)); + + Self { data: extra } + } } -const RESERVED_KEYS: &[&str] = &["image_file_name", "width", "height"]; +#[cfg(test)] +mod tests { + use super::*; + + use serde_json::json; -impl Serialize for ImageMetadata { - fn serialize(&self, serializer: S) -> Result { - let extra_count = self - .extra - .keys() - .filter(|k| !RESERVED_KEYS.contains(&k.as_str())) - .count(); - let mut map = serializer.serialize_map(Some(extra_count + 3))?; + #[test] + fn cli_managed_fields_override_sidecar_fields() { + let extra = serde_json::from_value(json!({ + (IMAGE_FILE_NAME_FIELD): "from-sidecar.png", + (WIDTH_FIELD): 1, + (HEIGHT_FIELD): 2, + "custom": "keep-me" + })) + .unwrap(); - // CLI-managed fields first - map.serialize_entry("image_file_name", &self.image_file_name)?; - map.serialize_entry("width", &self.width)?; - map.serialize_entry("height", &self.height)?; + let metadata = ImageMetadata::new("from-cli.png".to_owned(), 100, 200, extra); + let serialized = serde_json::to_value(metadata).unwrap(); - // User-provided sidecar fields, skipping any that conflict with CLI fields - for (key, value) in &self.extra { - if !RESERVED_KEYS.contains(&key.as_str()) { - map.serialize_entry(key, value)?; - } - } + let expected = json!({ + (IMAGE_FILE_NAME_FIELD): "from-cli.png", + (WIDTH_FIELD): 100, + (HEIGHT_FIELD): 200, + "custom": "keep-me" + }); - map.end() + assert_eq!(serialized, expected); } } diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 7d867821d5..bf49c5eb7b 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -330,12 +330,7 @@ fn upload_images( manifest_entries.insert( hash, - ImageMetadata { - extra, - image_file_name, - width: image.width, - height: image.height, - }, + ImageMetadata::new(image_file_name, image.width, image.height, extra), ); } From ba38bd1eed38b5ab0eb349bd9a9cd1f8261d3e74 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Tue, 3 Mar 2026 10:04:59 -0800 Subject: [PATCH 09/11] Add changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3780c14b0..a973ab1555 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Experimental Feature 🧑‍🔬 (internal-only) - Print snapshot URL after successful upload ([#3167](https://github.com/getsentry/sentry-cli/pull/3167)). +- Pipe snapshot sidecar metadata into upload as part of `sentry-cli build snapshots` command ([#3163](https://github.com/getsentry/sentry-cli/pull/3163)). ## 3.2.3 @@ -120,6 +121,7 @@ The following changes only apply when using `sentry-cli` via the npm package [`@ - Drop support for Node.js <18. The minimum required Node.js version is now 18.0.0 ([#2985](https://github.com/getsentry/sentry-cli/issues/2985)). - The type export `SentryCliReleases` has been removed. - The JavaScript wrapper now uses named exports instead of default exports ([#2989](https://github.com/getsentry/sentry-cli/pull/2989)). You need to update your imports: + ```js // Old (default import) const SentryCli = require('@sentry/cli'); @@ -129,6 +131,7 @@ The following changes only apply when using `sentry-cli` via the npm package [`@ ``` For ESM imports: + ```js // Old import SentryCli from '@sentry/cli'; @@ -137,7 +140,6 @@ The following changes only apply when using `sentry-cli` via the npm package [`@ import { SentryCli } from '@sentry/cli'; ``` - ### Improvements - The `sentry-cli upload-proguard` command now uses chunked uploading by default ([#2918](https://github.com/getsentry/sentry-cli/pull/2918)). Users who previously set the `SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD` environment variable to opt into this behavior no longer need to set the variable. @@ -164,6 +166,7 @@ The following changes only apply when using `sentry-cli` via the npm package [`@ - The `sentry-cli build upload` command now automatically detects the correct branch or tag reference in non-PR GitHub Actions workflows ([#2976](https://github.com/getsentry/sentry-cli/pull/2976)). Previously, `--head-ref` was only auto-detected for pull request workflows. Now it works for push, release, and other workflow types by using the `GITHUB_REF_NAME` environment variable. ### Fixes + - Fixed a bug where the `sentry-cli sourcemaps inject` command could inject JavaScript code into certain incorrectly formatted source map files, corrupting their JSON structure ([#3003](https://github.com/getsentry/sentry-cli/pull/3003)). ## 2.58.2 From 36a8ccaa2ba31a6cb71049c3487d4836a26e7736 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Wed, 4 Mar 2026 09:52:58 -0800 Subject: [PATCH 10/11] Feedback --- src/commands/build/snapshots.rs | 42 ++++++++++++++------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index bf49c5eb7b..0b8e65b3ee 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; -use std::fs; +use std::fs::{self, File}; +use std::io::BufReader; use std::path::{Path, PathBuf}; use std::str::FromStr as _; @@ -10,6 +11,7 @@ use itertools::Itertools as _; use log::{debug, info, warn}; use objectstore_client::{ClientBuilder, ExpirationPolicy, Usecase}; use secrecy::ExposeSecret as _; +use serde_json::Value; use sha2::{Digest as _, Sha256}; use walkdir::WalkDir; @@ -236,34 +238,23 @@ fn is_image_file(path: &Path) -> bool { /// /// For an image at `path/to/button.png`, looks for `path/to/button.json`. /// Returns a map of all key-value pairs from the JSON file. -fn read_sidecar_metadata(image_path: &Path) -> HashMap { +fn read_sidecar_metadata(image_path: &Path) -> Result> { let sidecar_path = image_path.with_extension("json"); if !sidecar_path.is_file() { - return HashMap::new(); + return Ok(HashMap::new()); } debug!("Reading sidecar metadata: {}", sidecar_path.display()); - let contents = match fs::read_to_string(&sidecar_path) { - Ok(c) => c, - Err(err) => { - warn!( - "Failed to read sidecar file {}: {err}", - sidecar_path.display() - ); - return HashMap::new(); - } - }; - match serde_json::from_str(&contents) { - Ok(map) => map, - Err(err) => { - warn!( - "Failed to parse sidecar file {}: {err}", - sidecar_path.display() - ); - HashMap::new() - } - } + let sidecar_file = File::open(&sidecar_path) + .with_context(|| format!("Failed to open sidecar file {}", sidecar_path.display()))?; + + serde_json::from_reader(BufReader::new(sidecar_file)).with_context(|| { + format!( + "Failed to read sidecar file {} as JSON", + sidecar_path.display() + ) + }) } fn upload_images( @@ -326,7 +317,10 @@ fn upload_images( .to_string_lossy() .into_owned(); - let extra = read_sidecar_metadata(&image.path); + let extra = read_sidecar_metadata(&image.path).unwrap_or_else(|err| { + warn!("Error reading sidecar metadata, ignoring it instead: {err:#}"); + HashMap::new() + }); manifest_entries.insert( hash, From 23fd2726f5e18c842e5534621a805895f71e84c0 Mon Sep 17 00:00:00 2001 From: Ryan Brooks Date: Wed, 4 Mar 2026 10:07:02 -0800 Subject: [PATCH 11/11] Cleanups --- CHANGELOG.md | 7 ++++++- src/api/data_types/snapshots.rs | 2 -- src/commands/build/snapshots.rs | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a973ab1555..1ce3b4f655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Experimental Feature 🧑‍🔬 (internal-only) + +- Pipe snapshot sidecar metadata into upload as part of `sentry-cli build snapshots` command ([#3163](https://github.com/getsentry/sentry-cli/pull/3163)). + ## 3.3.0 ### New Features @@ -13,7 +19,6 @@ ### Experimental Feature 🧑‍🔬 (internal-only) - Print snapshot URL after successful upload ([#3167](https://github.com/getsentry/sentry-cli/pull/3167)). -- Pipe snapshot sidecar metadata into upload as part of `sentry-cli build snapshots` command ([#3163](https://github.com/getsentry/sentry-cli/pull/3163)). ## 3.2.3 diff --git a/src/api/data_types/snapshots.rs b/src/api/data_types/snapshots.rs index 5e90778b78..ffce5f572a 100644 --- a/src/api/data_types/snapshots.rs +++ b/src/api/data_types/snapshots.rs @@ -29,8 +29,6 @@ pub struct SnapshotsManifest { // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py /// Metadata for a single image in a snapshot manifest. /// -/// Serializes as a flat JSON object. -/// /// CLI-managed fields (`image_file_name`, `width`, `height`) override any /// identically named fields provided by user sidecar metadata. #[derive(Debug, Serialize)] diff --git a/src/commands/build/snapshots.rs b/src/commands/build/snapshots.rs index 0b8e65b3ee..196ca51a27 100644 --- a/src/commands/build/snapshots.rs +++ b/src/commands/build/snapshots.rs @@ -351,3 +351,28 @@ fn upload_images( } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn make_image(width: u32, height: u32) -> ImageInfo { + ImageInfo { + path: PathBuf::from("img.png"), + relative_path: PathBuf::from("img.png"), + width, + height, + } + } + + #[test] + fn test_validate_image_sizes_at_limit_passes() { + assert!(validate_image_sizes(&[make_image(8000, 5000)]).is_ok()); + } + + #[test] + fn test_validate_image_sizes_over_limit_fails() { + let err = validate_image_sizes(&[make_image(8001, 5000)]).unwrap_err(); + assert!(err.to_string().contains("exceed the maximum pixel limit")); + } +}