Skip to content

Commit

Permalink
fix: Correctly handle debug meta in ser/de
Browse files Browse the repository at this point in the history
  • Loading branch information
mitsuhiko committed Mar 25, 2018
1 parent 87f2948 commit d24f8c8
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 11 deletions.
50 changes: 42 additions & 8 deletions src/protocol/v7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use protocol::utils::ts_seconds_float;

/// An arbitrary (JSON) value (`serde_json::value::Value`)
pub mod value {
pub use serde_json::value::{Value, Index, Number, from_value, to_value};
pub use serde_json::value::{Map, Value, Index, Number, from_value, to_value};
}

/// The internally use arbitrary data map type (`linked_hash_map::LinkedHashMap`)
Expand Down Expand Up @@ -198,6 +198,12 @@ impl From<i64> for ThreadId {
}
}

impl From<i32> for ThreadId {
fn from(id: i32) -> ThreadId {
ThreadId::Int(id as u64)
}
}

impl From<u32> for ThreadId {
fn from(id: u32) -> ThreadId {
ThreadId::Int(id as u64)
Expand All @@ -223,6 +229,13 @@ impl fmt::Display for ThreadId {
#[derive(Default, Debug, Clone, PartialEq, Eq, Ord, PartialOrd, Hash)]
pub struct Addr(pub u64);

impl Addr {
/// Returns `true` if this address is the null pointer.
pub fn is_null(&self) -> bool {
self.0 == 0
}
}

impl fmt::Display for Addr {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "0x{:x}", self.0)
Expand All @@ -235,6 +248,12 @@ impl From<u64> for Addr {
}
}

impl From<i32> for Addr {
fn from(addr: i32) -> Addr {
Addr(addr as u64)
}
}

impl From<u32> for Addr {
fn from(addr: u32) -> Addr {
Addr(addr as u64)
Expand Down Expand Up @@ -460,7 +479,7 @@ pub struct SystemSdkInfo {
/// the major version of the SDK as integer or 0
pub version_major: u32,
/// the minor version of the SDK as integer or 0
pub version_minior: u32,
pub version_minor: u32,
/// the patch version of the SDK as integer or 0
pub version_patchlevel: u32,
}
Expand Down Expand Up @@ -506,6 +525,7 @@ pub struct AppleDebugImage {
/// The size of the image in bytes.
pub image_size: u64,
/// The address where the image is loaded at runtime.
#[serde(skip_serializing_if = "Addr::is_null")]
pub image_vmaddr: Addr,
/// The unique UUID of the image.
pub uuid: Uuid,
Expand All @@ -526,8 +546,17 @@ pub struct DebugMeta {
#[serde(skip_serializing_if = "Option::is_none")]
pub sdk_info: Option<SystemSdkInfo>,
/// A list of debug information files.
#[serde(skip_serializing_if = "Option::is_none")]
pub images: Option<DebugImage>,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub images: Vec<DebugImage>,
}

impl DebugMeta {
/// Returns true if the debug meta is empty.
///
/// This is used by the serializer to entirely skip the section.
pub fn is_empty(&self) -> bool {
self.sdk_info.is_none() && self.images.is_empty()
}
}

/// Represents a repository reference.
Expand Down Expand Up @@ -643,8 +672,8 @@ pub struct Event {
#[serde(skip_serializing_if = "Map::is_empty")]
pub extra: Map<String, Value>,
/// Debug meta information.
#[serde(skip_serializing_if = "Option::is_none")]
pub debug_meta: Option<DebugMeta>,
#[serde(skip_serializing_if = "DebugMeta::is_empty")]
pub debug_meta: DebugMeta,
/// SDK metadata
#[serde(skip_serializing_if = "Option::is_none")]
pub sdk_info: Option<ClientSdkInfo>,
Expand Down Expand Up @@ -689,7 +718,7 @@ impl Default for Event {
threads: Vec::new(),
tags: Map::new(),
extra: Map::new(),
debug_meta: None,
debug_meta: Default::default(),
sdk_info: None,
other: Map::new(),
}
Expand Down Expand Up @@ -1006,7 +1035,12 @@ impl Serialize for DebugImage {
where
S: Serializer,
{
let mut c = match to_value(self).map_err(S::Error::custom)? {
let actual = match *self {
DebugImage::Apple(ref img) => to_value(img),
DebugImage::Proguard(ref img) => to_value(img),
DebugImage::Unknown(ref img) => to_value(img),
};
let mut c = match actual.map_err(S::Error::custom)? {
Value::Object(map) => map,
_ => unreachable!(),
};
Expand Down
146 changes: 143 additions & 3 deletions tests/test_protocol_v7.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
extern crate sentry_types;
extern crate serde;
extern crate serde_json;
#[macro_use] extern crate serde_json;
extern crate uuid;
extern crate chrono;

Expand Down Expand Up @@ -380,7 +380,7 @@ fn test_threads() {
let event = v7::Event {
threads: vec![
v7::Thread {
id: Some(42u32.into()),
id: Some(42.into()),
name: Some("Awesome Thread".into()),
crashed: true,
current: true,
Expand All @@ -396,6 +396,51 @@ fn test_threads() {
"{\"threads\":{\"values\":[{\"id\":42,\"name\":\"Awesome Thread\",\
\"crashed\":true,\"current\":true}]}}"
);

let event = v7::Event {
threads: vec![
v7::Thread {
stacktrace: Some(v7::Stacktrace {
frames: vec![
v7::Frame {
function: Some("main".into()),
location: v7::FileLocation {
filename: Some("hello.py".into()),
line: Some(1),
..Default::default()
},
..Default::default()
},
],
..Default::default()
}),
raw_stacktrace: Some(v7::Stacktrace {
frames: vec![
v7::Frame {
function: Some("main".into()),
location: v7::FileLocation {
filename: Some("hello.py".into()),
line: Some(1),
..Default::default()
},
..Default::default()
},
],
..Default::default()
}),
..Default::default()
}
],
..Default::default()
};

assert_roundtrip(&event);
assert_eq!(
serde_json::to_string(&event).unwrap(),
"{\"threads\":{\"values\":[{\"stacktrace\":{\"frames\":[{\"function\":\
\"main\",\"filename\":\"hello.py\",\"lineno\":1}]},\"raw_stacktrace\"\
:{\"frames\":[{\"function\":\"main\",\"filename\":\"hello.py\",\"lineno\":1}]}}]}}"
);
}

#[test]
Expand Down Expand Up @@ -470,6 +515,101 @@ fn test_request() {
);
}

#[test]
fn test_tags() {
let event = v7::Event {
tags: {
let mut m = v7::Map::new();
m.insert("device_type".into(), "mobile".into());
m.insert("interpreter".into(), "7".into());
m
},
..Default::default()
};

assert_roundtrip(&event);
assert_eq!(
serde_json::to_string(&event).unwrap(),
"{\"tags\":{\"device_type\":\"mobile\",\"interpreter\":\"7\"}}"
);
}

#[test]
fn test_extra() {
let event = v7::Event {
extra: {
let mut m = v7::Map::new();
m.insert("component_state".into(), json!({
"dirty": true,
"revision": 17
}));
m
},
..Default::default()
};

assert_roundtrip(&event);
assert_eq!(
serde_json::to_string(&event).unwrap(),
"{\"extra\":{\"component_state\":{\"dirty\":true,\"revision\":17}}}"
);
}

#[test]
fn test_debug_meta() {
let event = v7::Event {
debug_meta: v7::DebugMeta {
sdk_info: Some(v7::SystemSdkInfo {
sdk_name: "iOS".into(),
version_major: 10,
version_minor: 3,
version_patchlevel: 0,
}),
..Default::default()
},
..Default::default()
};

assert_roundtrip(&event);
assert_eq!(
serde_json::to_string(&event).unwrap(),
"{\"debug_meta\":{\"sdk_info\":{\"sdk_name\":\"iOS\",\"version_major\":10,\
\"version_minor\":3,\"version_patchlevel\":0}}}"
);

let event = v7::Event {
debug_meta: v7::DebugMeta {
images: vec![
v7::DebugImage::Apple(v7::AppleDebugImage {
name: "CoreFoundation".into(),
arch: Some("arm64".into()),
cpu_type: Some(1233),
cpu_subtype: Some(3),
image_addr: 0.into(),
image_size: 4096,
image_vmaddr: 32768.into(),
uuid: "494f3aea-88fa-4296-9644-fa8ef5d139b6".parse().unwrap(),
}),
v7::DebugImage::Proguard(v7::ProguardDebugImage {
uuid: "8c954262-f905-4992-8a61-f60825f4553b".parse().unwrap(),
})
],
..Default::default()
},
..Default::default()
};

assert_roundtrip(&event);
assert_eq!(
serde_json::to_string(&event).unwrap(),
"{\"debug_meta\":{\"images\":[{\"name\":\"CoreFoundation\",\"arch\":\
\"arm64\",\"cpu_type\":1233,\"cpu_subtype\":3,\"image_addr\":\"0x0\",\
\"image_size\":4096,\"image_vmaddr\":\"0x8000\",\"uuid\":\
\"494f3aea-88fa-4296-9644-fa8ef5d139b6\",\"type\":\"apple\"},\
{\"uuid\":\"8c954262-f905-4992-8a61-f60825f4553b\",\"type\":\"proguard\"}]}}"
);
}

#[test]
fn test_canonical_exception() {
let mut event: v7::Event = Default::default();
Expand Down Expand Up @@ -691,7 +831,7 @@ fn test_addr_format() {
fn test_addr_api() {
use std::ptr;
assert_eq!(v7::Addr::from(42u64), v7::Addr(42));
assert_eq!(v7::Addr::from(42u32), v7::Addr(42));
assert_eq!(v7::Addr::from(42), v7::Addr(42));
assert_eq!(v7::Addr::from(ptr::null::<()>()), v7::Addr(0));
}

Expand Down

0 comments on commit d24f8c8

Please sign in to comment.