Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Unsupported variant to CacheError and ObjectFileStatus #1541

Merged
merged 4 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Loading