Skip to content

Commit

Permalink
feat: Add Unsupported variant to CacheError and ObjectFileStatus
Browse files Browse the repository at this point in the history
Second try of #1541. That PR was reverted in #1551 because of deployment
issues which turned out to be unrelated.
  • Loading branch information
loewenheim committed Nov 20, 2024
1 parent 263886e commit c9bff79
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 7 deletions.
1 change: 1 addition & 0 deletions crates/symbolicator-js/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ impl From<CacheError> for JsScrapingResult {
),
CacheError::DownloadError(details) => (JsScrapingFailureReason::DownloadError, details),
CacheError::Malformed(details) => (JsScrapingFailureReason::Other, details),
CacheError::Unsupported(details) => (JsScrapingFailureReason::Other, details),
CacheError::InternalError => (JsScrapingFailureReason::Other, String::new()),
};

Expand Down
10 changes: 7 additions & 3 deletions crates/symbolicator-native/src/caches/ppdb_caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl PortablePdbCacheActor {
let found_object = self
.objects
.find(FindObject {
filetypes: &[FileType::PortablePdb],
filetypes: &[FileType::PortablePdb, FileType::Pdb],
identifier: request.identifier,
sources: request.sources,
scope: request.scope,
Expand Down Expand Up @@ -124,8 +124,12 @@ fn write_ppdb_cache(file: &mut File, object_handle: &ObjectHandle) -> CacheEntry

let ppdb_obj = match object_handle.object() {
Object::PortablePdb(ppdb_obj) => ppdb_obj,
// FIXME(swatinem): instead of panic, we should return an internal error?
_ => panic!("object handle does not contain a valid portable pdb object"),
_ => {
tracing::warn!("Trying to symbolicate a .NET event with a non-PPDB object file");
return Err(CacheError::Unsupported(
"Only portable PDB files can be used for .NET symbolication".to_owned(),
));
}
};

tracing::debug!("Converting ppdb cache for {}", object_handle.cache_key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub fn object_file_status_from_cache_entry<T>(cache_entry: &CacheEntry<T>) -> Ob
}
Err(CacheError::Timeout(_)) => ObjectFileStatus::Timeout,
Err(CacheError::Malformed(_)) => ObjectFileStatus::Malformed,
Err(CacheError::Unsupported(_)) => ObjectFileStatus::Unsupported,
Err(CacheError::InternalError) => ObjectFileStatus::Other,
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: crates/symbolicator-service/tests/integration/symbolication.rs
source: crates/symbolicator-native/tests/integration/symbolication.rs
expression: response.unwrap()
---
stacktraces:
Expand Down Expand Up @@ -37,6 +37,10 @@ modules:
location: "http://localhost:<port>/symbols/portable-embedded.pdb/B6919861510C48879994943F64F70C37870b9ef9/portable-embedded.src.zip"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/portable-embedded.pdb/B6919861510C48879994943F64F70C37ffffffff/portable-embedded.pd_"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/portable-embedded.pdb/B6919861510C48879994943F64F70C37ffffffff/portable-embedded.pdb"
download:
Expand All @@ -52,4 +56,3 @@ modules:
location: "http://localhost:<port>/symbols/portable-embedded.pdb/B6919861510C48879994943F64F70C37ffffffff/portable-embedded.src.zip"
download:
status: notfound

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: crates/symbolicator-service/tests/integration/symbolication.rs
source: crates/symbolicator-native/tests/integration/symbolication.rs
expression: response.unwrap()
---
stacktraces:
Expand Down Expand Up @@ -66,6 +66,10 @@ modules:
location: "http://localhost:<port>/symbols/integration.pdb/0C1033F78632492E91C6C314B72E1920e60b819d/integration.src.zip"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/integration.pdb/0C1033F78632492E91C6C314B72E1920ffffffff/integration.pd_"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/integration.pdb/0C1033F78632492E91C6C314B72E1920ffffffff/integration.pdb"
download:
Expand All @@ -81,4 +85,3 @@ modules:
location: "http://localhost:<port>/symbols/integration.pdb/0C1033F78632492E91C6C314B72E1920ffffffff/integration.src.zip"
download:
status: notfound

Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ modules:
location: "http://localhost:<port>/symbols/source-links.pdb/0C380A12822140698565BEE6B3AC196Ea596286e/source-links.src.zip"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/source-links.pdb/0C380A12822140698565BEE6B3AC196Effffffff/source-links.pd_"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/source-links.pdb/0C380A12822140698565BEE6B3AC196Effffffff/source-links.pdb"
download:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ modules:
location: "http://localhost:<port>/symbols/source-links.pdb/37E9E8A61A8E404EB93C6902E277FF55a09672e1/source-links.src.zip"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/source-links.pdb/37E9E8A61A8E404EB93C6902E277FF55ffffffff/source-links.pd_"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/source-links.pdb/37E9E8A61A8E404EB93C6902E277FF55ffffffff/source-links.pdb"
download:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
source: crates/symbolicator-native/tests/integration/symbolication.rs
expression: response.unwrap()
---
stacktraces:
- frames:
- status: missing
original_index: 0
addr_mode: "rel:0"
instruction_addr: "0xa"
function_id: "0x6"
- status: missing
original_index: 1
addr_mode: "rel:0"
instruction_addr: "0x6"
function_id: "0x5"
- status: missing
original_index: 2
addr_mode: "rel:0"
instruction_addr: "0x0"
function_id: "0x3"
- status: missing
original_index: 3
addr_mode: "rel:0"
instruction_addr: "0x0"
function_id: "0x2"
- status: missing
original_index: 4
addr_mode: "rel:0"
instruction_addr: "0x2d"
function_id: "0x1"
modules:
- debug_status: unsupported
features:
has_debug_info: true
has_unwind_info: true
has_symbols: true
has_sources: false
arch: unknown
type: pe_dotnet
debug_id: 3249d99d-0c40-4931-8610-f4e4fb0b6936
debug_file: crash.pdb
image_addr: "0x0"
candidates:
- source: local
location: "http://localhost:<port>/symbols/crash.pdb/3249D99D0C4049318610F4E4FB0B69360/crash.src.zip"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/crash.pdb/3249D99D0C4049318610F4E4FB0B6936ffffffff/crash.pd_"
download:
status: notfound
- source: local
location: "http://localhost:<port>/symbols/crash.pdb/3249D99D0C4049318610F4E4FB0B6936ffffffff/crash.pdb"
download:
status: ok
features:
has_debug_info: true
has_unwind_info: true
has_symbols: true
has_sources: false
debug:
status: error
details: "unsupported: Only portable PDB files can be used for .NET symbolication"
- source: local
location: "http://localhost:<port>/symbols/crash.pdb/3249D99D0C4049318610F4E4FB0B6936ffffffff/crash.src.zip"
download:
status: notfound
42 changes: 42 additions & 0 deletions crates/symbolicator-native/tests/integration/symbolication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,48 @@ async fn test_dotnet_integration() {
assert_snapshot!(response.unwrap());
}

#[tokio::test]
async fn test_dotnet_windows_pdb() {
let (symbolication, _cache_dir) = setup_service(|_| ());
let (_srv, source) = symbol_server();

// try to symbolicate a `PE_DOTNET` event with a Windows PDB file, this should fail
let request = make_symbolication_request(
vec![source],
r#"[{
"type":"pe_dotnet",
"debug_file":"crash.pdb",
"debug_id":"3249d99d0c4049318610f4e4fb0b6936"
}]"#,
r#"[{
"frames":[{
"instruction_addr": 10,
"function_id": 6,
"addr_mode":"rel:0"
}, {
"instruction_addr": 6,
"function_id": 5,
"addr_mode": "rel:0"
}, {
"instruction_addr": 0,
"function_id": 3,
"addr_mode": "rel:0"
}, {
"instruction_addr": 0,
"function_id": 2,
"addr_mode": "rel:0"
}, {
"instruction_addr": 45,
"function_id": 1,
"addr_mode": "rel:0"
}]
}]"#,
);
let response = symbolication.symbolicate(request).await;

assert_snapshot!(response.unwrap());
}

#[tokio::test]
async fn test_dotnet_embedded_sources() {
let (symbolication, _cache_dir) = setup_service(|_| ());
Expand Down
16 changes: 16 additions & 0 deletions crates/symbolicator-service/src/caching/cache_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ pub enum CacheError {
/// during symcache conversion
#[error("malformed: {0}")]
Malformed(String),
/// The object is of a type that cannot be used for the symbolication task it was
/// requested for.
///
/// This is currently only used when we try to symbolicate a .NET event with a Windows
/// PDB file. A tracking issue in `symbolic` for supporting this case is
/// [here](https://github.com/getsentry/symbolic/issues/871).
#[error("unsupported: {0}")]
Unsupported(String),
/// An unexpected error in symbolicator itself.
///
/// This variant is not intended to be persisted to or read from caches.
Expand All @@ -64,6 +72,7 @@ impl CacheError {
pub(super) const PERMISSION_DENIED_MARKER: &'static [u8] = b"permissiondenied";
pub(super) const TIMEOUT_MARKER: &'static [u8] = b"timeout";
pub(super) const DOWNLOAD_ERROR_MARKER: &'static [u8] = b"downloaderror";
pub(super) const UNSUPPORTED_MARKER: &'static [u8] = b"unsupported";

/// Writes error markers and details to a file.
///
Expand Down Expand Up @@ -99,6 +108,10 @@ impl CacheError {
file.write_all(Self::DOWNLOAD_ERROR_MARKER).await?;
file.write_all(details.as_bytes()).await?;
}
CacheError::Unsupported(details) => {
file.write_all(Self::UNSUPPORTED_MARKER).await?;
file.write_all(details.as_bytes()).await?;
}
CacheError::InternalError => {
unreachable!("this was already handled above");
}
Expand Down Expand Up @@ -134,6 +147,9 @@ impl CacheError {
} else if let Some(raw_message) = bytes.strip_prefix(Self::MALFORMED_MARKER) {
let err_msg = utf8_message(raw_message);
Some(Self::Malformed(err_msg.into_owned()))
} else if let Some(raw_message) = bytes.strip_prefix(Self::UNSUPPORTED_MARKER) {
let err_msg = utf8_message(raw_message);
Some(Self::Unsupported(err_msg.into_owned()))
} else if bytes.is_empty() {
Some(Self::NotFound)
} else {
Expand Down
7 changes: 7 additions & 0 deletions crates/symbolicator-service/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ pub enum ObjectFileStatus {
FetchingFailed,
/// Downloading or processing the file took too long.
Timeout,
/// The file could not be used for the purpose for which it was requested.
///
/// This is currently only used when we try to symbolicate a .NET event with a Windows
/// PDB file. A tracking issue in `symbolic` for supporting this case is
/// [here](https://github.com/getsentry/symbolic/issues/871).
Unsupported,
/// An internal error while handling this image.
Other,
}
Expand All @@ -161,6 +167,7 @@ impl ObjectFileStatus {
ObjectFileStatus::Malformed => "malformed",
ObjectFileStatus::FetchingFailed => "fetching_failed",
ObjectFileStatus::Timeout => "timeout",
ObjectFileStatus::Unsupported => "unsupported",
ObjectFileStatus::Other => "other",
}
}
Expand Down
Binary file not shown.

0 comments on commit c9bff79

Please sign in to comment.