feat(difs): Add debug status to dif candidates info

.github/workflows/ci.yml

@@ -28,7 +28,7 @@ jobs:
         run: pip install --upgrade black flake8
 
       - name: Run Black
-        run: black --check tests
+        run: black --check --diff tests
 
       - name: Run Flake8
         run: flake8 tests

CHANGELOG.md

@@ -4,7 +4,7 @@
 
 ### Features
 
-- Symbolication responses include download information about all DIF object files which were looked up on the available object sources. ([#309](https://github.com/getsentry/symbolicator/pull/309))
+- Symbolication responses include download information about all DIF object files which were looked up on the available object sources. ([#309](https://github.com/getsentry/symbolicator/pull/309) [#316](https://github.com/getsentry/symbolicator/pull/316))
 
 ### Bug Fixes
 

src/actors/objects/mod.rs

@@ -21,7 +21,9 @@ use crate::cache::{Cache, CacheKey, CacheStatus};
 use crate::logging::LogError;
 use crate::services::download::{DownloadError, DownloadService, DownloadStatus};
 use crate::sources::{FileType, SourceConfig, SourceFileId, SourceId, SourceLocation};
-use crate::types::{ObjectCandidate, ObjectDownloadInfo, ObjectFeatures, ObjectId, Scope};
+use crate::types::{
+    AllObjectCandidates, ObjectCandidate, ObjectDownloadInfo, ObjectFeatures, ObjectId, Scope,
+};
 use crate::utils::futures::ThreadPool;
 use crate::utils::sentry::{SentryFutureExt, WriteSentryScope};
 
@@ -421,6 +423,18 @@ impl ObjectFileMeta {
     pub fn features(&self) -> ObjectFeatures {
         self.features
     }
+
+    pub fn source(&self) -> &SourceId {
+        self.request.file_id.source_id()
+    }
+
+    pub fn location(&self) -> SourceLocation {
+        self.request.file_id.location()
+    }
+
+    pub fn status(&self) -> CacheStatus {
+        self.status
+    }
 }
 
 /// Handle to local cache file of an object.
@@ -544,7 +558,7 @@ pub struct FoundObject {
     pub meta: Option<Arc<ObjectFileMeta>>,
     /// This is a list of some meta information on all objects which have been considered
     /// for this object.  It could be populated even if no matching object is found.
-    pub candidates: Vec<ObjectCandidate>,
+    pub candidates: AllObjectCandidates,
 }
 
 impl ObjectsActor {
@@ -736,7 +750,7 @@ impl ObjectsActor {
                     })
                     .map(|(_i, response)| response)
                     .transpose()
-                    .map(|meta| FoundObject { meta, candidates })
+                    .map(|meta| FoundObject { meta, candidates: candidates.into() })
             },
         )
     }

src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap

@@ -38,6 +38,8 @@ modules:
             has_unwind_info: false
             has_symbols: true
             has_sources: false
+        debug:
+          status: ok
       - source: local
         location: 502F/C0A5/1EC1/3E47/9998/684FA139DCA7.app
         download:

src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap

@@ -143,15 +143,15 @@ modules:
     image_size: 106496
     candidates:
       - source: local
-        location: f1/c3bcc0279865fe3058404b2831d9e64135386c.debug
+        location: crash/C0BCC3F19827FE653058404B2831D9E60/crash.sym
         download:
           status: notfound
       - source: local
         location: f1/c3bcc0279865fe3058404b2831d9e64135386c
         download:
           status: notfound
       - source: local
-        location: crash/C0BCC3F19827FE653058404B2831D9E60/crash.sym
+        location: f1/c3bcc0279865fe3058404b2831d9e64135386c.debug
         download:
           status: notfound
   - debug_status: unused
@@ -186,11 +186,11 @@ modules:
     image_size: 1835008
     candidates:
       - source: local
-        location: b5/381a457906d279073822a5ceb24c4bfef94ddb.debug
+        location: b5/381a457906d279073822a5ceb24c4bfef94ddb
         download:
           status: notfound
       - source: local
-        location: b5/381a457906d279073822a5ceb24c4bfef94ddb
+        location: b5/381a457906d279073822a5ceb24c4bfef94ddb.debug
         download:
           status: notfound
       - source: local
@@ -259,11 +259,11 @@ modules:
     image_size: 155648
     candidates:
       - source: local
-        location: 5d/7b6259552275a3c17bd4c3fd05f5a6bf40caa5.debug
+        location: 5d/7b6259552275a3c17bd4c3fd05f5a6bf40caa5
         download:
           status: notfound
       - source: local
-        location: 5d/7b6259552275a3c17bd4c3fd05f5a6bf40caa5
+        location: 5d/7b6259552275a3c17bd4c3fd05f5a6bf40caa5.debug
         download:
           status: notfound
       - source: local
@@ -287,11 +287,11 @@ modules:
     image_size: 8192
     candidates:
       - source: local
-        location: 6c/5f1875b9048fb4b8dfd832e74ad31a9aafb38f.debug
+        location: 6c/5f1875b9048fb4b8dfd832e74ad31a9aafb38f
         download:
           status: notfound
       - source: local
-        location: 6c/5f1875b9048fb4b8dfd832e74ad31a9aafb38f
+        location: 6c/5f1875b9048fb4b8dfd832e74ad31a9aafb38f.debug
         download:
           status: notfound
       - source: local

src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap

@@ -170,26 +170,28 @@ modules:
     image_size: 36864
     candidates:
       - source: local
-        location: crash.pdb/3249D99D0C4049318610F4E4FB0B69361/crash.pdb
-        download:
-          status: ok
-          features:
-            has_debug_info: true
-            has_unwind_info: true
-            has_symbols: true
-            has_sources: false
-      - source: local
-        location: crash.pdb/3249D99D0C4049318610F4E4FB0B69361/crash.pd_
+        location: crash.exe/5AB380779000/crash.ex_
         download:
           status: notfound
       - source: local
         location: crash.exe/5AB380779000/crash.exe
         download:
           status: notfound
       - source: local
-        location: crash.exe/5AB380779000/crash.ex_
+        location: crash.pdb/3249D99D0C4049318610F4E4FB0B69361/crash.pd_
         download:
           status: notfound
+      - source: local
+        location: crash.pdb/3249D99D0C4049318610F4E4FB0B69361/crash.pdb
+        download:
+          status: ok
+          features:
+            has_debug_info: true
+            has_unwind_info: true
+            has_symbols: true
+            has_sources: false
+        debug:
+          status: ok
       - source: local
         location: crash.pdb/3249D99D0C4049318610F4E4FB0B69361/crash.sym
         download:
@@ -361,19 +363,19 @@ modules:
     image_size: 917504
     candidates:
       - source: local
-        location: wkernel32.pdb/D347455996F747D6BF43C176B2171E681/wkernel32.pdb
+        location: kernel32.dll/590285E9e0000/kernel32.dl_
         download:
           status: notfound
       - source: local
-        location: wkernel32.pdb/D347455996F747D6BF43C176B2171E681/wkernel32.pd_
+        location: kernel32.dll/590285E9e0000/kernel32.dll
         download:
           status: notfound
       - source: local
-        location: kernel32.dll/590285E9e0000/kernel32.dll
+        location: wkernel32.pdb/D347455996F747D6BF43C176B2171E681/wkernel32.pd_
         download:
           status: notfound
       - source: local
-        location: kernel32.dll/590285E9e0000/kernel32.dl_
+        location: wkernel32.pdb/D347455996F747D6BF43C176B2171E681/wkernel32.pdb
         download:
           status: notfound
       - source: local
@@ -457,19 +459,19 @@ modules:
     image_size: 1585152
     candidates:
       - source: local
-        location: wntdll.pdb/971F98E5CE6041FFB2D7235BBEB345781/wntdll.pdb
+        location: ntdll.dll/59B0D8F3183000/ntdll.dl_
         download:
           status: notfound
       - source: local
-        location: wntdll.pdb/971F98E5CE6041FFB2D7235BBEB345781/wntdll.pd_
+        location: ntdll.dll/59B0D8F3183000/ntdll.dll
         download:
           status: notfound
       - source: local
-        location: ntdll.dll/59B0D8F3183000/ntdll.dll
+        location: wntdll.pdb/971F98E5CE6041FFB2D7235BBEB345781/wntdll.pd_
         download:
           status: notfound
       - source: local
-        location: ntdll.dll/59B0D8F3183000/ntdll.dl_
+        location: wntdll.pdb/971F98E5CE6041FFB2D7235BBEB345781/wntdll.pdb
         download:
           status: notfound
       - source: local

src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap

@@ -38,6 +38,8 @@ modules:
             has_unwind_info: false
             has_symbols: true
             has_sources: false
+        debug:
+          status: ok
       - source: local
         location: 502F/C0A5/1EC1/3E47/9998/684FA139DCA7.app
         download:

src/actors/symbolication.rs

@@ -673,9 +673,7 @@ impl SymCacheLookup {
                     entry.object_info.arch = symcache.arch();
                     entry.object_info.features.merge(symcache.features());
 
-                    let mut difs = Vec::new();
-                    difs.extend_from_slice(&symcache.candidates());
-                    entry.object_info.candidates = difs; // TODO(flub): merge!
+                    entry.object_info.candidates = symcache.candidates(); // TODO(flub): merge!
                 }
 
                 entry.symcache = symcache;

src/actors/symcaches.rs

@@ -17,7 +17,9 @@ use crate::actors::objects::{
 };
 use crate::cache::{Cache, CacheKey, CacheStatus};
 use crate::sources::{FileType, SourceConfig};
-use crate::types::{ObjectCandidate, ObjectFeatures, ObjectId, ObjectType, Scope};
+use crate::types::{
+    AllObjectCandidates, ObjectFeatures, ObjectId, ObjectType, ObjectUseInfo, Scope,
+};
 use crate::utils::futures::ThreadPool;
 use crate::utils::sentry::{SentryFutureExt, WriteSentryScope};
 
@@ -75,7 +77,7 @@ pub struct SymCacheFile {
     features: ObjectFeatures,
     status: CacheStatus,
     arch: Arch,
-    candidates: Arc<[ObjectCandidate]>,
+    candidates: AllObjectCandidates,
 }
 
 impl SymCacheFile {
@@ -100,7 +102,7 @@ impl SymCacheFile {
     }
 
     /// Returns the list of DIFs which were searched for this symcache.
-    pub fn candidates(&self) -> Arc<[ObjectCandidate]> {
+    pub fn candidates(&self) -> AllObjectCandidates {
         self.candidates.clone()
     }
 }
@@ -111,7 +113,7 @@ struct FetchSymCacheInternal {
     objects_actor: ObjectsActor,
     object_meta: Arc<ObjectFileMeta>,
     threadpool: ThreadPool,
-    candidates: Arc<[ObjectCandidate]>,
+    candidates: AllObjectCandidates,
 }
 
 impl CacheItemRequest for FetchSymCacheInternal {
@@ -184,6 +186,31 @@ impl CacheItemRequest for FetchSymCacheInternal {
             .map(|cache| cache.arch())
             .unwrap_or_default();
 
+        // If self.object_meta.status() was != Positive than that status got passed straight
+        // through to our own `status` argument.
+        let debug = match status {
+            CacheStatus::Positive => ObjectUseInfo::Ok,
+            CacheStatus::Negative => {
+                if self.object_meta.status() == CacheStatus::Positive {
+                    ObjectUseInfo::Error {
+                        details: String::from("Object file no longer available"),
+                    }
+                } else {
+                    // No need to pretend that we were going to use this symcache if the
+                    // original object file was already not there, that status is already
+                    // reported.
+                    ObjectUseInfo::None
+                }
+            }
+            CacheStatus::Malformed => ObjectUseInfo::Malformed,
+        };
+        let mut candidates = self.candidates.clone(); // yuk!
+        candidates.set_debug(
+            self.object_meta.source().clone(),
+            self.object_meta.location(),
+            debug,
+        );
+
         SymCacheFile {
             object_type: self.request.object_type,
             identifier: self.request.identifier.clone(),
@@ -192,7 +219,7 @@ impl CacheItemRequest for FetchSymCacheInternal {
             features: self.object_meta.features(),
             status,
             arch,
-            candidates: self.candidates.clone(),
+            candidates,
         }
     }
 }
@@ -232,7 +259,6 @@ impl SymCacheActor {
 
         find_result_future.and_then(move |find_result: FoundObject| {
             let FoundObject { meta, candidates } = find_result;
-            let candidates: Arc<[ObjectCandidate]> = Arc::from(candidates);
             meta.map(clone!(candidates, |object_meta| {
                 Either::A(symcaches.compute_memoized(FetchSymCacheInternal {
                     request,

src/sources.rs

@@ -15,7 +15,7 @@ use crate::utils::sentry::WriteSentryScope;
 /// An identifier for DIF sources.
 ///
 /// This is essentially a newtype for a string.
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 pub struct SourceId(String);
 
 // For now we allow this to be unused, some tests use these already.
@@ -103,7 +103,7 @@ impl WriteSentryScope for SourceConfig {
 ///
 /// It is essentially a `/`-separated string. This is currently used by all sources other than
 /// [`SentrySourceConfig`]. This may change in the future.
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 pub struct SourceLocation(String);
 
 impl SourceLocation {