diff --git a/cli/examples/custom-commit-templater/main.rs b/cli/examples/custom-commit-templater/main.rs
index ac90684c52..4b1c67c770 100644
--- a/cli/examples/custom-commit-templater/main.rs
+++ b/cli/examples/custom-commit-templater/main.rs
@@ -75,6 +75,7 @@ impl MostDigitsInId {
                 .evaluate_programmatic(repo)
                 .unwrap()
                 .iter()
+                .map(Result::unwrap)
                 .map(|id| num_digits_in_id(&id))
                 .max()
                 .unwrap_or(0)
@@ -102,6 +103,7 @@ impl PartialSymbolResolver for TheDigitestResolver {
                 .evaluate_programmatic(repo)
                 .map_err(|err| RevsetResolutionError::Other(err.into()))?
                 .iter()
+                .map(Result::unwrap)
                 .filter(|id| num_digits_in_id(id) == self.cache.count(repo))
                 .collect_vec(),
         ))
diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index 4dbc32f7ec..9ce97f3216 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -762,6 +762,7 @@ impl WorkspaceCommandEnvironment {
         })?;
 
         if let Some(commit_id) = commit_id_iter.next() {
+            let commit_id = commit_id?;
             let error = if &commit_id == repo.store().root_commit_id() {
                 user_error(format!(
                     "The root commit {} is immutable",
diff --git a/cli/src/commands/bookmark/list.rs b/cli/src/commands/bookmark/list.rs
index cffd0e83e4..b8802ae781 100644
--- a/cli/src/commands/bookmark/list.rs
+++ b/cli/src/commands/bookmark/list.rs
@@ -14,6 +14,7 @@
 
 use std::collections::HashSet;
 
+use itertools::Itertools;
 use jj_lib::git;
 use jj_lib::revset::RevsetExpression;
 use jj_lib::str_util::StringPattern;
@@ -100,7 +101,8 @@ pub fn cmd_bookmark_list(
             // Intersects with the set of local bookmark targets to minimize the lookup
             // space.
             expression.intersect_with(&RevsetExpression::bookmarks(StringPattern::everything()));
-            let filtered_targets: HashSet<_> = expression.evaluate_to_commit_ids()?.collect();
+            let filtered_targets: HashSet<_> =
+                expression.evaluate_to_commit_ids()?.try_collect()?;
             bookmark_names.extend(
                 view.local_bookmarks()
                     .filter(|(_, target)| {
diff --git a/cli/src/commands/debug/revset.rs b/cli/src/commands/debug/revset.rs
index 55c309e2b4..0afffd1606 100644
--- a/cli/src/commands/debug/revset.rs
+++ b/cli/src/commands/debug/revset.rs
@@ -15,6 +15,7 @@
 use std::fmt::Debug;
 use std::io::Write as _;
 
+use jj_lib::object_id::ObjectId;
 use jj_lib::revset;
 use jj_lib::revset::RevsetDiagnostics;
 
@@ -68,7 +69,7 @@ pub fn cmd_debug_revset(
 
     writeln!(ui.stdout(), "-- Commit IDs:")?;
     for commit_id in revset.iter() {
-        writeln!(ui.stdout(), "{commit_id}")?;
+        writeln!(ui.stdout(), "{}", commit_id?.hex())?;
     }
     Ok(())
 }
diff --git a/cli/src/commands/duplicate.rs b/cli/src/commands/duplicate.rs
index 7ae929f747..c229ca0c46 100644
--- a/cli/src/commands/duplicate.rs
+++ b/cli/src/commands/duplicate.rs
@@ -15,6 +15,7 @@
 use std::io::Write;
 
 use indexmap::IndexMap;
+use itertools::Itertools;
 use jj_lib::backend::CommitId;
 use jj_lib::commit::Commit;
 use jj_lib::repo::Repo;
@@ -48,7 +49,7 @@ pub(crate) fn cmd_duplicate(
     let to_duplicate: Vec<CommitId> = workspace_command
         .parse_union_revsets(ui, &args.revisions)?
         .evaluate_to_commit_ids()?
-        .collect(); // in reverse topological order
+        .try_collect()?; // in reverse topological order
     if to_duplicate.is_empty() {
         writeln!(ui.status(), "No revisions to duplicate.")?;
         return Ok(());
diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs
index f720326a1e..7982830f30 100644
--- a/cli/src/commands/fix.rs
+++ b/cli/src/commands/fix.rs
@@ -150,7 +150,7 @@ pub(crate) fn cmd_fix(
         workspace_command.parse_union_revsets(ui, &args.source)?
     }
     .evaluate_to_commit_ids()?
-    .collect();
+    .try_collect()?;
     workspace_command.check_rewritable(root_commits.iter())?;
     let matcher = workspace_command
         .parse_file_patterns(ui, &args.paths)?
diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs
index 429d02b22d..72ae3f0691 100644
--- a/cli/src/commands/git/push.rs
+++ b/cli/src/commands/git/push.rs
@@ -612,7 +612,9 @@ fn find_bookmarks_targeted_by_revisions<'a>(
         .intersection(&RevsetExpression::bookmarks(StringPattern::everything()));
         let current_bookmarks_revset = current_bookmarks_expression
             .evaluate_programmatic(workspace_command.repo().as_ref())?;
-        revision_commit_ids.extend(current_bookmarks_revset.iter());
+        for commit_id in current_bookmarks_revset.iter() {
+            revision_commit_ids.insert(commit_id?);
+        }
         if revision_commit_ids.is_empty() {
             writeln!(
                 ui.warning_default(),
@@ -631,7 +633,9 @@ fn find_bookmarks_targeted_by_revisions<'a>(
                 "No bookmarks point to the specified revisions: {rev_arg}"
             )?;
         }
-        revision_commit_ids.extend(commit_ids);
+        for commit_id in commit_ids {
+            revision_commit_ids.insert(commit_id?);
+        }
     }
     let bookmarks_targeted = workspace_command
         .repo()
diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs
index 8c783ef120..060dbb811b 100644
--- a/cli/src/commands/log.rs
+++ b/cli/src/commands/log.rs
@@ -17,6 +17,7 @@ use jj_lib::graph::GraphEdgeType;
 use jj_lib::graph::ReverseGraphIterator;
 use jj_lib::graph::TopoGroupedGraphIterator;
 use jj_lib::repo::Repo;
+use jj_lib::revset::RevsetEvaluationError;
 use jj_lib::revset::RevsetExpression;
 use jj_lib::revset::RevsetFilterPredicate;
 use jj_lib::revset::RevsetIteratorExt;
@@ -173,11 +174,13 @@ pub(crate) fn cmd_log(
             let mut graph = get_graphlog(graph_style, raw_output.as_mut());
             let forward_iter = TopoGroupedGraphIterator::new(revset.iter_graph());
             let iter: Box<dyn Iterator<Item = _>> = if args.reversed {
-                Box::new(ReverseGraphIterator::new(forward_iter))
+                Box::new(ReverseGraphIterator::new(forward_iter)?)
             } else {
                 Box::new(forward_iter)
             };
-            for (commit_id, edges) in iter.take(limit) {
+            for node in iter.take(limit) {
+                let (commit_id, edges) = node?;
+
                 // The graph is keyed by (CommitId, is_synthetic)
                 let mut graphlog_edges = vec![];
                 // TODO: Should we update revset.iter_graph() to yield this flag instead of all
@@ -255,11 +258,12 @@ pub(crate) fn cmd_log(
                 }
             }
         } else {
-            let iter: Box<dyn Iterator<Item = CommitId>> = if args.reversed {
-                Box::new(revset.iter().reversed())
-            } else {
-                Box::new(revset.iter())
-            };
+            let iter: Box<dyn Iterator<Item = Result<CommitId, RevsetEvaluationError>>> =
+                if args.reversed {
+                    Box::new(revset.iter().reversed()?)
+                } else {
+                    Box::new(revset.iter())
+                };
             for commit_or_error in iter.commits(store).take(limit) {
                 let commit = commit_or_error?;
                 with_content_format
diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs
index 09ea51a7a0..d08531b0ef 100644
--- a/cli/src/commands/new.rs
+++ b/cli/src/commands/new.rs
@@ -243,6 +243,7 @@ fn ensure_no_commit_loop(
         .iter()
         .next()
     {
+        let commit_id = commit_id?;
         return Err(user_error(format!(
             "Refusing to create a loop: commit {} would be both an ancestor and a descendant of \
              the new commit",
diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs
index 654761013b..f06a70d957 100644
--- a/cli/src/commands/operation/diff.rs
+++ b/cli/src/commands/operation/diff.rs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use std::collections::HashMap;
+use std::convert::Infallible;
 use std::sync::Arc;
 
 use indexmap::IndexMap;
@@ -198,19 +199,21 @@ pub fn show_op_diff(
             let mut raw_output = formatter.raw()?;
             let mut graph = get_graphlog(graph_style, raw_output.as_mut());
 
-            let graph_iter =
-                TopoGroupedGraphIterator::new(ordered_change_ids.iter().map(|change_id| {
+            let graph_iter = TopoGroupedGraphIterator::new(ordered_change_ids.iter().map(
+                |change_id| -> Result<_, Infallible> {
                     let parent_change_ids = change_parents.get(change_id).unwrap();
-                    (
+                    Ok((
                         change_id.clone(),
                         parent_change_ids
                             .iter()
                             .map(|parent_change_id| GraphEdge::direct(parent_change_id.clone()))
                             .collect_vec(),
-                    )
-                }));
+                    ))
+                },
+            ));
 
-            for (change_id, edges) in graph_iter {
+            for node in graph_iter {
+                let (change_id, edges) = node.unwrap();
                 let modified_change = changes.get(&change_id).unwrap();
                 let edges = edges
                     .iter()
diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs
index 1e6ac96d40..041b81142a 100644
--- a/cli/src/commands/rebase.rs
+++ b/cli/src/commands/rebase.rs
@@ -622,6 +622,7 @@ fn ensure_no_commit_loop(
         .iter()
         .next()
     {
+        let commit_id = commit_id?;
         return Err(user_error(format!(
             "Refusing to create a loop: commit {} would be both an ancestor and a descendant of \
              the rebased commits",
diff --git a/cli/src/commands/simplify_parents.rs b/cli/src/commands/simplify_parents.rs
index bc556e5e7f..5700344a97 100644
--- a/cli/src/commands/simplify_parents.rs
+++ b/cli/src/commands/simplify_parents.rs
@@ -48,10 +48,10 @@ pub(crate) fn cmd_simplify_parents(
             .parse_union_revsets(ui, &args.revisions)?
             .expression(),
     );
-    let commit_ids = workspace_command
+    let commit_ids: Vec<_> = workspace_command
         .attach_revset_evaluator(revs)
         .evaluate_to_commit_ids()?
-        .collect_vec();
+        .try_collect()?;
     workspace_command.check_rewritable(&commit_ids)?;
     let commit_ids_set: HashSet<_> = commit_ids.iter().cloned().collect();
     let num_orig_commits = commit_ids.len();
diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs
index 25d0e87e79..98d1090fcd 100644
--- a/cli/src/commands/status.rs
+++ b/cli/src/commands/status.rs
@@ -111,7 +111,7 @@ pub(crate) fn cmd_status(
             let wc_revset = RevsetExpression::commit(wc_commit.id().clone());
 
             // Ancestors with conflicts, excluding the current working copy commit.
-            let ancestors_conflicts = workspace_command
+            let ancestors_conflicts: Vec<_> = workspace_command
                 .attach_revset_evaluator(
                     wc_revset
                         .parents()
@@ -120,7 +120,7 @@ pub(crate) fn cmd_status(
                         .minus(&workspace_command.env().immutable_expression()),
                 )
                 .evaluate_to_commit_ids()?
-                .collect();
+                .try_collect()?;
 
             workspace_command.report_repo_conflicts(formatter, repo, ancestors_conflicts)?;
         } else {
diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs
index 397cf07ed2..9c068e8844 100644
--- a/cli/src/revset_util.rs
+++ b/cli/src/revset_util.rs
@@ -105,7 +105,10 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
     /// sorted in reverse topological order.
     pub fn evaluate_to_commit_ids(
         &self,
-    ) -> Result<Box<dyn Iterator<Item = CommitId> + 'repo>, UserRevsetEvaluationError> {
+    ) -> Result<
+        Box<dyn Iterator<Item = Result<CommitId, RevsetEvaluationError>> + 'repo>,
+        UserRevsetEvaluationError,
+    > {
         Ok(self.evaluate()?.iter())
     }
 
diff --git a/lib/src/annotate.rs b/lib/src/annotate.rs
index 8d8bc276eb..6231066531 100644
--- a/lib/src/annotate.rs
+++ b/lib/src/annotate.rs
@@ -36,6 +36,7 @@ use crate::graph::GraphEdgeType;
 use crate::merged_tree::MergedTree;
 use crate::repo::Repo;
 use crate::repo_path::RepoPath;
+use crate::revset::RevsetEvaluationError;
 use crate::revset::RevsetExpression;
 use crate::revset::RevsetFilterPredicate;
 use crate::store::Store;
@@ -132,7 +133,7 @@ pub fn get_annotation_for_file(
     repo: &dyn Repo,
     starting_commit: &Commit,
     file_path: &RepoPath,
-) -> Result<AnnotateResults, BackendError> {
+) -> Result<AnnotateResults, RevsetEvaluationError> {
     let original_contents =
         get_file_contents(starting_commit.store(), file_path, &starting_commit.tree()?)?;
     let num_lines = original_contents.split_inclusive(|b| *b == b'\n').count();
@@ -154,7 +155,7 @@ fn process_commits(
     starting_commit_id: &CommitId,
     file_name: &RepoPath,
     num_lines: usize,
-) -> Result<OriginalLineMap, BackendError> {
+) -> Result<OriginalLineMap, RevsetEvaluationError> {
     let predicate = RevsetFilterPredicate::File(FilesetExpression::file_path(file_name.to_owned()));
     let revset = RevsetExpression::commit(starting_commit_id.clone())
         .union(
@@ -167,7 +168,8 @@ fn process_commits(
     let mut commit_line_map = get_initial_commit_line_map(starting_commit_id, num_lines);
     let mut original_line_map = HashMap::new();
 
-    for (cid, edge_list) in revset.iter_graph() {
+    for node in revset.iter_graph() {
+        let (cid, edge_list) = node?;
         let current_commit = repo.store().get_commit(&cid)?;
         process_commit(
             repo,
diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs
index 3c6d562dc0..e51a7a28b8 100644
--- a/lib/src/default_index/revset_engine.rs
+++ b/lib/src/default_index/revset_engine.rs
@@ -49,6 +49,7 @@ use crate::default_index::AsCompositeIndex;
 use crate::default_index::CompositeIndex;
 use crate::default_index::IndexPosition;
 use crate::graph::GraphEdge;
+use crate::graph::GraphNodeResult;
 use crate::matchers::Matcher;
 use crate::matchers::Visit;
 use crate::merged_tree::resolve_file_values;
@@ -130,11 +131,12 @@ impl<I: AsCompositeIndex + Clone> RevsetImpl<I> {
     pub fn iter_graph_impl(
         &self,
         skip_transitive_edges: bool,
-    ) -> impl Iterator<Item = (CommitId, Vec<GraphEdge<CommitId>>)> {
+    ) -> impl Iterator<Item = Result<(CommitId, Vec<GraphEdge<CommitId>>), RevsetEvaluationError>>
+    {
         let index = self.index.clone();
         let walk = self.inner.positions();
         let mut graph_walk = RevsetGraphWalk::new(walk, skip_transitive_edges);
-        iter::from_fn(move || graph_walk.next(index.as_composite()))
+        iter::from_fn(move || graph_walk.next(index.as_composite()).map(Ok))
     }
 }
 
@@ -147,7 +149,7 @@ impl<I> fmt::Debug for RevsetImpl<I> {
 }
 
 impl<I: AsCompositeIndex + Clone> Revset for RevsetImpl<I> {
-    fn iter<'a>(&self) -> Box<dyn Iterator<Item = CommitId> + 'a>
+    fn iter<'a>(&self) -> Box<dyn Iterator<Item = Result<CommitId, RevsetEvaluationError>> + 'a>
     where
         Self: 'a,
     {
@@ -156,11 +158,13 @@ impl<I: AsCompositeIndex + Clone> Revset for RevsetImpl<I> {
         Box::new(iter::from_fn(move || {
             let index = index.as_composite();
             let pos = walk.next(index)?;
-            Some(index.entry_by_pos(pos).commit_id())
+            Some(Ok(index.entry_by_pos(pos).commit_id()))
         }))
     }
 
-    fn commit_change_ids<'a>(&self) -> Box<dyn Iterator<Item = (CommitId, ChangeId)> + 'a>
+    fn commit_change_ids<'a>(
+        &self,
+    ) -> Box<dyn Iterator<Item = Result<(CommitId, ChangeId), RevsetEvaluationError>> + 'a>
     where
         Self: 'a,
     {
@@ -170,11 +174,13 @@ impl<I: AsCompositeIndex + Clone> Revset for RevsetImpl<I> {
             let index = index.as_composite();
             let pos = walk.next(index)?;
             let entry = index.entry_by_pos(pos);
-            Some((entry.commit_id(), entry.change_id()))
+            Some(Ok((entry.commit_id(), entry.change_id())))
         }))
     }
 
-    fn iter_graph<'a>(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<GraphEdge<CommitId>>)> + 'a>
+    fn iter_graph<'a>(
+        &self,
+    ) -> Box<dyn Iterator<Item = GraphNodeResult<CommitId, RevsetEvaluationError>> + 'a>
     where
         Self: 'a,
     {
diff --git a/lib/src/git.rs b/lib/src/git.rs
index 3c2699242a..bd30b1ec95 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -381,6 +381,7 @@ fn abandon_unreachable_commits(
         .evaluate_programmatic(mut_repo)
         .unwrap()
         .iter()
+        .map(Result::unwrap) // TODO: Return error to caller
         .collect_vec();
     for abandoned_commit in &abandoned_commits {
         mut_repo.record_abandoned_commit(abandoned_commit.clone());
diff --git a/lib/src/graph.rs b/lib/src/graph.rs
index 9b1067ab06..af061720be 100644
--- a/lib/src/graph.rs
+++ b/lib/src/graph.rs
@@ -19,12 +19,16 @@ use std::collections::HashSet;
 use std::collections::VecDeque;
 use std::hash::Hash;
 
+use crate::revset::RevsetEvaluationError;
+
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 pub struct GraphEdge<N> {
     pub target: N,
     pub edge_type: GraphEdgeType,
 }
 
+pub type GraphNodeResult<N, E> = Result<(N, Vec<GraphEdge<N>>), E>;
+
 impl<N> GraphEdge<N> {
     pub fn missing(target: N) -> Self {
         Self {
@@ -77,10 +81,13 @@ impl<N> ReverseGraphIterator<N>
 where
     N: Hash + Eq + Clone,
 {
-    pub fn new(input: impl IntoIterator<Item = (N, Vec<GraphEdge<N>>)>) -> Self {
+    pub fn new(
+        input: impl Iterator<Item = Result<(N, Vec<GraphEdge<N>>), RevsetEvaluationError>>,
+    ) -> Result<Self, RevsetEvaluationError> {
         let mut entries = vec![];
         let mut reverse_edges: HashMap<N, Vec<GraphEdge<N>>> = HashMap::new();
-        for (node, edges) in input {
+        for item in input {
+            let (node, edges) = item?;
             for GraphEdge { target, edge_type } in edges {
                 reverse_edges.entry(target).or_default().push(GraphEdge {
                     target: node.clone(),
@@ -95,15 +102,15 @@ where
             let edges = reverse_edges.get(&node).cloned().unwrap_or_default();
             items.push((node, edges));
         }
-        Self { items }
+        Ok(Self { items })
     }
 }
 
 impl<N> Iterator for ReverseGraphIterator<N> {
-    type Item = (N, Vec<GraphEdge<N>>);
+    type Item = Result<(N, Vec<GraphEdge<N>>), RevsetEvaluationError>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.items.pop()
+        self.items.pop().map(Ok)
     }
 }
 
@@ -148,10 +155,12 @@ impl<N> Default for TopoGroupedGraphNode<N> {
     }
 }
 
-impl<N, I> TopoGroupedGraphIterator<N, I>
+type NextNodeResult<N, E> = Result<Option<(N, Vec<GraphEdge<N>>)>, E>;
+
+impl<N, E, I> TopoGroupedGraphIterator<N, I>
 where
     N: Clone + Hash + Eq,
-    I: Iterator<Item = (N, Vec<GraphEdge<N>>)>,
+    I: Iterator<Item = Result<(N, Vec<GraphEdge<N>>), E>>,
 {
     /// Wraps the given iterator to group topological branches. The input
     /// iterator must be topologically ordered.
@@ -165,9 +174,16 @@ where
         }
     }
 
-    #[must_use]
-    fn populate_one(&mut self) -> Option<()> {
-        let (current_id, edges) = self.input_iter.next()?;
+    fn populate_one(&mut self) -> Result<Option<()>, E> {
+        let (current_id, edges) = match self.input_iter.next() {
+            Some(Ok(data)) => data,
+            Some(Err(err)) => {
+                return Err(err);
+            }
+            None => {
+                return Ok(None);
+            }
+        };
 
         // Set up reverse reference
         for parent_id in reachable_targets(&edges) {
@@ -189,7 +205,7 @@ where
             self.new_head_ids.push_back(current_id);
         }
 
-        Some(())
+        Ok(Some(()))
     }
 
     /// Enqueues the first new head which will unblock the waiting ancestors.
@@ -246,8 +262,7 @@ where
         self.emittable_ids.push(new_head_id);
     }
 
-    #[must_use]
-    fn next_node(&mut self) -> Option<(N, Vec<GraphEdge<N>>)> {
+    fn next_node(&mut self) -> NextNodeResult<N, E> {
         // Based on Kahn's algorithm
         loop {
             if let Some(current_id) = self.emittable_ids.last() {
@@ -264,7 +279,7 @@ where
                 }
                 let Some(edges) = current_node.edges.take() else {
                     // Not yet populated
-                    self.populate_one().expect("parent node should exist");
+                    self.populate_one()?.expect("parent node should exist");
                     continue;
                 };
                 // The second (or the last) parent will be visited first
@@ -281,36 +296,42 @@ where
                         self.blocked_ids.insert(parent_id.clone());
                     }
                 }
-                return Some((current_id, edges));
+                return Ok(Some((current_id, edges)));
             } else if !self.new_head_ids.is_empty() {
                 self.flush_new_head();
             } else {
                 // Populate the first or orphan head
-                self.populate_one()?;
+                if self.populate_one()?.is_none() {
+                    return Ok(None);
+                }
             }
         }
     }
 }
 
-impl<N, I> Iterator for TopoGroupedGraphIterator<N, I>
+impl<N, E, I> Iterator for TopoGroupedGraphIterator<N, I>
 where
     N: Clone + Hash + Eq,
-    I: Iterator<Item = (N, Vec<GraphEdge<N>>)>,
+    I: Iterator<Item = Result<(N, Vec<GraphEdge<N>>), E>>,
 {
-    type Item = (N, Vec<GraphEdge<N>>);
+    type Item = Result<(N, Vec<GraphEdge<N>>), E>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        if let Some(node) = self.next_node() {
-            Some(node)
-        } else {
-            assert!(self.nodes.is_empty(), "all nodes should have been emitted");
-            None
+        match self.next_node() {
+            Ok(Some(node)) => Some(Ok(node)),
+            Ok(None) => {
+                assert!(self.nodes.is_empty(), "all nodes should have been emitted");
+                None
+            }
+            Err(err) => Some(Err(err)),
         }
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use std::convert::Infallible;
+
     use itertools::Itertools as _;
     use renderdag::Ancestor;
     use renderdag::GraphRowRenderer;
@@ -384,13 +405,19 @@ mod tests {
         "###);
     }
 
-    fn topo_grouped<I>(graph_iter: I) -> TopoGroupedGraphIterator<char, I::IntoIter>
+    fn topo_grouped<I, E>(graph_iter: I) -> TopoGroupedGraphIterator<char, I::IntoIter>
     where
-        I: IntoIterator<Item = (char, Vec<GraphEdge<char>>)>,
+        I: IntoIterator<Item = Result<(char, Vec<GraphEdge<char>>), E>>,
     {
         TopoGroupedGraphIterator::new(graph_iter.into_iter())
     }
 
+    fn infallible(
+        input: (char, Vec<GraphEdge<char>>),
+    ) -> Result<(char, Vec<GraphEdge<char>>), Infallible> {
+        Ok(input)
+    }
+
     #[test]
     fn test_topo_grouped_multiple_roots() {
         let graph = [
@@ -409,7 +436,7 @@ mod tests {
 
         A
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         C  missing(Y)
         │
         ~
@@ -422,11 +449,11 @@ mod tests {
         "###);
 
         // All nodes can be lazily emitted.
-        let mut iter = topo_grouped(graph.iter().cloned().peekable());
-        assert_eq!(iter.next().unwrap().0, 'C');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'B');
-        assert_eq!(iter.next().unwrap().0, 'B');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'A');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible).peekable());
+        assert_eq!(iter.next().unwrap().unwrap().0, 'C');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'B');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'B');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'A');
     }
 
     #[test]
@@ -452,7 +479,7 @@ mod tests {
         "###);
         // D-A is found earlier than B-A, but B is emitted first because it belongs to
         // the emitting branch.
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         E  direct(B)
         │
         │ C  direct(B)
@@ -466,13 +493,13 @@ mod tests {
         "###);
 
         // E can be lazy, then D and C will be queued.
-        let mut iter = topo_grouped(graph.iter().cloned().peekable());
-        assert_eq!(iter.next().unwrap().0, 'E');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'D');
-        assert_eq!(iter.next().unwrap().0, 'C');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'B');
-        assert_eq!(iter.next().unwrap().0, 'B');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'A');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible).peekable());
+        assert_eq!(iter.next().unwrap().unwrap().0, 'E');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'D');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'C');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'B');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'B');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'A');
     }
 
     #[test]
@@ -499,7 +526,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         F  direct(D)
         │
         D  direct(B)
@@ -515,13 +542,13 @@ mod tests {
         "###);
 
         // F can be lazy, then E will be queued, then C.
-        let mut iter = topo_grouped(graph.iter().cloned().peekable());
-        assert_eq!(iter.next().unwrap().0, 'F');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'E');
-        assert_eq!(iter.next().unwrap().0, 'D');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'C');
-        assert_eq!(iter.next().unwrap().0, 'E');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'B');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible).peekable());
+        assert_eq!(iter.next().unwrap().unwrap().0, 'F');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'E');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'D');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'C');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'E');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'B');
     }
 
     #[test]
@@ -557,7 +584,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         I  direct(E)
         │
         │ F  direct(E)
@@ -579,11 +606,11 @@ mod tests {
         "###);
 
         // I can be lazy, then H, G, and F will be queued.
-        let mut iter = topo_grouped(graph.iter().cloned().peekable());
-        assert_eq!(iter.next().unwrap().0, 'I');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'H');
-        assert_eq!(iter.next().unwrap().0, 'F');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'E');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible).peekable());
+        assert_eq!(iter.next().unwrap().unwrap().0, 'I');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'H');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'F');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'E');
     }
 
     #[test]
@@ -627,7 +654,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         I  direct(A)
         │
         │ B  direct(A)
@@ -692,7 +719,7 @@ mod tests {
 
         "###);
         // A::F is picked at A, and A will be unblocked. Then, C::D at C, ...
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         G  direct(A)
         │
         │ F  direct(C)
@@ -744,7 +771,7 @@ mod tests {
 
         "###);
         // A::K is picked at A, and A will be unblocked. Then, H::I at H, ...
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         L  direct(A)
         │
         │ K  direct(H)
@@ -807,7 +834,7 @@ mod tests {
 
         "###);
         // A::K is picked at A, and A will be unblocked. Then, E::G at E, ...
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         L  direct(A)
         │
         │ K  direct(E)
@@ -870,7 +897,7 @@ mod tests {
         "###);
         // K-E,J is resolved without queuing new heads. Then, G::H, F::I, B::C, and
         // A::D.
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         K    direct(E), direct(J)
         ├─╮
         │ J  direct(G)
@@ -935,7 +962,7 @@ mod tests {
         "###);
         // K-I,J is resolved without queuing new heads. Then, D::F, B::H, C::E, and
         // A::G.
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         K    direct(I), direct(J)
         ├─╮
         │ J  direct(D)
@@ -988,7 +1015,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         F  direct(E)
         │
         E    direct(C), direct(D)
@@ -1004,15 +1031,15 @@ mod tests {
         "###);
 
         // F, E, and D can be lazy, then C will be queued, then B.
-        let mut iter = topo_grouped(graph.iter().cloned().peekable());
-        assert_eq!(iter.next().unwrap().0, 'F');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'E');
-        assert_eq!(iter.next().unwrap().0, 'E');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'D');
-        assert_eq!(iter.next().unwrap().0, 'D');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'C');
-        assert_eq!(iter.next().unwrap().0, 'B');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'A');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible).peekable());
+        assert_eq!(iter.next().unwrap().unwrap().0, 'F');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'E');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'E');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'D');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'D');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'C');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'B');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'A');
     }
 
     #[test]
@@ -1042,7 +1069,7 @@ mod tests {
           A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         E  direct(D)
         │
         D    missing(Y), direct(C)
@@ -1062,15 +1089,15 @@ mod tests {
         "###);
 
         // All nodes can be lazily emitted.
-        let mut iter = topo_grouped(graph.iter().cloned().peekable());
-        assert_eq!(iter.next().unwrap().0, 'E');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'D');
-        assert_eq!(iter.next().unwrap().0, 'D');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'C');
-        assert_eq!(iter.next().unwrap().0, 'C');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'B');
-        assert_eq!(iter.next().unwrap().0, 'B');
-        assert_eq!(iter.input_iter.peek().unwrap().0, 'A');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible).peekable());
+        assert_eq!(iter.next().unwrap().unwrap().0, 'E');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'D');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'D');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'C');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'C');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'B');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'B');
+        assert_eq!(iter.input_iter.peek().unwrap().as_ref().unwrap().0, 'A');
     }
 
     #[test]
@@ -1100,7 +1127,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         G  direct(E)
         │
         E    direct(B), direct(C)
@@ -1148,7 +1175,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         H  direct(F)
         │
         F  direct(D)
@@ -1189,7 +1216,7 @@ mod tests {
 
         "###);
         // A is emitted first because it's the second parent.
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         D  direct(C)
         │
         C    direct(B), direct(A)
@@ -1242,7 +1269,7 @@ mod tests {
 
         "###);
         // Second branches are visited first.
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         J    direct(I), direct(G)
         ├─╮
         │ G  direct(D)
@@ -1302,7 +1329,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         J  direct(F)
         │
         F  direct(C)
@@ -1364,7 +1391,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         J  direct(F)
         │
         F  direct(D)
@@ -1413,7 +1440,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         E  direct(C)
         │
         C  direct(A)
@@ -1463,7 +1490,7 @@ mod tests {
         "###);
         // Topological order must be preserved. Depending on the implementation,
         // E might be requested more than once by paths D->E and B->D->E.
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         I    direct(H), direct(G)
         ├─╮
         │ G  direct(B)
@@ -1500,7 +1527,7 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         C    direct(A), direct(B)
         ├─╮
         │ B  direct(A)
@@ -1512,12 +1539,12 @@ mod tests {
         // A is queued once by C-A because B isn't populated at this point. Since
         // B is the second parent, B-A is processed next and A is queued again. So
         // one of them in the queue has to be ignored.
-        let mut iter = topo_grouped(graph.iter().cloned());
-        assert_eq!(iter.next().unwrap().0, 'C');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible));
+        assert_eq!(iter.next().unwrap().unwrap().0, 'C');
         assert_eq!(iter.emittable_ids, vec!['A', 'B']);
-        assert_eq!(iter.next().unwrap().0, 'B');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'B');
         assert_eq!(iter.emittable_ids, vec!['A', 'A']);
-        assert_eq!(iter.next().unwrap().0, 'A');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'A');
         assert!(iter.next().is_none());
         assert!(iter.emittable_ids.is_empty());
     }
@@ -1533,17 +1560,17 @@ mod tests {
         A
 
         "###);
-        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned())), @r###"
+        insta::assert_snapshot!(format_graph(topo_grouped(graph.iter().cloned().map(infallible)).map(Result::unwrap)), @r###"
         B  direct(A), direct(A)
         │
         A
 
         "###);
 
-        let mut iter = topo_grouped(graph.iter().cloned());
-        assert_eq!(iter.next().unwrap().0, 'B');
+        let mut iter = topo_grouped(graph.iter().cloned().map(infallible));
+        assert_eq!(iter.next().unwrap().unwrap().0, 'B');
         assert_eq!(iter.emittable_ids, vec!['A', 'A']);
-        assert_eq!(iter.next().unwrap().0, 'A');
+        assert_eq!(iter.next().unwrap().unwrap().0, 'A');
         assert!(iter.next().is_none());
         assert!(iter.emittable_ids.is_empty());
     }
diff --git a/lib/src/id_prefix.rs b/lib/src/id_prefix.rs
index 0e2f1f39f8..435878124c 100644
--- a/lib/src/id_prefix.rs
+++ b/lib/src/id_prefix.rs
@@ -40,9 +40,9 @@ use crate::revset::SymbolResolverExtension;
 #[derive(Debug, Error)]
 pub enum IdPrefixIndexLoadError {
     #[error("Failed to resolve short-prefixes disambiguation revset")]
-    Resolution(#[source] RevsetResolutionError),
+    Resolution(#[from] RevsetResolutionError),
     #[error("Failed to evaluate short-prefixes disambiguation revset")]
-    Evaluation(#[source] RevsetEvaluationError),
+    Evaluation(#[from] RevsetEvaluationError),
 }
 
 struct DisambiguationData {
@@ -67,13 +67,10 @@ impl DisambiguationData {
             let resolved_expression = self
                 .expression
                 .clone()
-                .resolve_user_expression(repo, &symbol_resolver)
-                .map_err(IdPrefixIndexLoadError::Resolution)?;
-            let revset = resolved_expression
-                .evaluate(repo)
-                .map_err(IdPrefixIndexLoadError::Evaluation)?;
+                .resolve_user_expression(repo, &symbol_resolver)?;
+            let revset = resolved_expression.evaluate(repo)?;
 
-            let commit_change_ids = revset.commit_change_ids().collect_vec();
+            let commit_change_ids: Vec<_> = revset.commit_change_ids().try_collect()?;
             let mut commit_index = IdIndex::with_capacity(commit_change_ids.len());
             let mut change_index = IdIndex::with_capacity(commit_change_ids.len());
             for (i, (commit_id, change_id)) in commit_change_ids.iter().enumerate() {
diff --git a/lib/src/repo.rs b/lib/src/repo.rs
index 382ec96f5e..9810f64c79 100644
--- a/lib/src/repo.rs
+++ b/lib/src/repo.rs
@@ -1169,7 +1169,8 @@ impl MutableRepo {
         let heads_to_add = heads_to_add_expression
             .evaluate_programmatic(self)
             .unwrap()
-            .iter();
+            .iter()
+            .map(Result::unwrap); // TODO: Return error to caller
 
         let mut view = self.view().store_view().clone();
         for commit_id in self.parent_mapping.keys() {
@@ -1782,6 +1783,8 @@ impl MutableRepo {
         for (commit_id, change_id) in revset::walk_revs(self, old_heads, new_heads)
             .unwrap()
             .commit_change_ids()
+            .map(Result::unwrap)
+        // TODO: Return error to caller
         {
             removed_changes
                 .entry(change_id)
@@ -1797,6 +1800,8 @@ impl MutableRepo {
         for (commit_id, change_id) in revset::walk_revs(self, new_heads, old_heads)
             .unwrap()
             .commit_change_ids()
+            .map(Result::unwrap)
+        // TODO: Return error to caller
         {
             if let Some(old_commits) = removed_changes.get(&change_id) {
                 for old_commit in old_commits {
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 568c56e82c..ce045d590a 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -37,7 +37,7 @@ use crate::dsl_util::AliasExpandError as _;
 use crate::fileset;
 use crate::fileset::FilesetDiagnostics;
 use crate::fileset::FilesetExpression;
-use crate::graph::GraphEdge;
+use crate::graph::GraphNodeResult;
 use crate::hex_util::to_forward_hex;
 use crate::id_prefix::IdPrefixContext;
 use crate::id_prefix::IdPrefixIndex;
@@ -92,7 +92,7 @@ pub enum RevsetResolutionError {
 #[derive(Debug, Error)]
 pub enum RevsetEvaluationError {
     #[error("Unexpected error from store")]
-    StoreError(#[source] BackendError),
+    StoreError(#[from] BackendError),
     #[error(transparent)]
     Other(Box<dyn std::error::Error + Send + Sync>),
 }
@@ -2197,16 +2197,20 @@ impl VisibilityResolutionContext<'_> {
 
 pub trait Revset: fmt::Debug {
     /// Iterate in topological order with children before parents.
-    fn iter<'a>(&self) -> Box<dyn Iterator<Item = CommitId> + 'a>
+    fn iter<'a>(&self) -> Box<dyn Iterator<Item = Result<CommitId, RevsetEvaluationError>> + 'a>
     where
         Self: 'a;
 
     /// Iterates commit/change id pairs in topological order.
-    fn commit_change_ids<'a>(&self) -> Box<dyn Iterator<Item = (CommitId, ChangeId)> + 'a>
+    fn commit_change_ids<'a>(
+        &self,
+    ) -> Box<dyn Iterator<Item = Result<(CommitId, ChangeId), RevsetEvaluationError>> + 'a>
     where
         Self: 'a;
 
-    fn iter_graph<'a>(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<GraphEdge<CommitId>>)> + 'a>
+    fn iter_graph<'a>(
+        &self,
+    ) -> Box<dyn Iterator<Item = GraphNodeResult<CommitId, RevsetEvaluationError>> + 'a>
     where
         Self: 'a;
 
@@ -2230,10 +2234,10 @@ pub trait Revset: fmt::Debug {
 
 pub trait RevsetIteratorExt<'index, I> {
     fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I>;
-    fn reversed(self) -> ReverseRevsetIterator;
+    fn reversed(self) -> Result<ReverseRevsetIterator, RevsetEvaluationError>;
 }
 
-impl<I: Iterator<Item = CommitId>> RevsetIteratorExt<'_, I> for I {
+impl<I: Iterator<Item = Result<CommitId, RevsetEvaluationError>>> RevsetIteratorExt<'_, I> for I {
     fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I> {
         RevsetCommitIterator {
             iter: self,
@@ -2241,10 +2245,9 @@ impl<I: Iterator<Item = CommitId>> RevsetIteratorExt<'_, I> for I {
         }
     }
 
-    fn reversed(self) -> ReverseRevsetIterator {
-        ReverseRevsetIterator {
-            entries: self.into_iter().collect_vec(),
-        }
+    fn reversed(self) -> Result<ReverseRevsetIterator, RevsetEvaluationError> {
+        let entries: Vec<_> = self.into_iter().try_collect()?;
+        Ok(ReverseRevsetIterator { entries })
     }
 }
 
@@ -2253,11 +2256,14 @@ pub struct RevsetCommitIterator<I> {
     iter: I,
 }
 
-impl<I: Iterator<Item = CommitId>> Iterator for RevsetCommitIterator<I> {
+impl<I: Iterator<Item = Result<CommitId, RevsetEvaluationError>>> Iterator
+    for RevsetCommitIterator<I>
+{
     type Item = Result<Commit, RevsetEvaluationError>;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.iter.next().map(|commit_id| {
+            let commit_id = commit_id?;
             self.store
                 .get_commit(&commit_id)
                 .map_err(RevsetEvaluationError::StoreError)
@@ -2270,10 +2276,10 @@ pub struct ReverseRevsetIterator {
 }
 
 impl Iterator for ReverseRevsetIterator {
-    type Item = CommitId;
+    type Item = Result<CommitId, RevsetEvaluationError>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.entries.pop()
+        self.entries.pop().map(Ok)
     }
 }
 
diff --git a/lib/tests/test_default_revset_graph_iterator.rs b/lib/tests/test_default_revset_graph_iterator.rs
index fe896b052e..2106ef7b99 100644
--- a/lib/tests/test_default_revset_graph_iterator.rs
+++ b/lib/tests/test_default_revset_graph_iterator.rs
@@ -77,7 +77,10 @@ fn test_graph_iterator_linearized(skip_transitive_edges: bool) {
     let root_commit = repo.store().root_commit();
 
     let revset = revset_for_commits(repo.as_ref(), &[&commit_a, &commit_d]);
-    let commits = revset.iter_graph_impl(skip_transitive_edges).collect_vec();
+    let commits: Vec<_> = revset
+        .iter_graph_impl(skip_transitive_edges)
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 2);
     assert_eq!(commits[0].0, *commit_d.id());
     assert_eq!(commits[1].0, *commit_a.id());
@@ -114,7 +117,10 @@ fn test_graph_iterator_virtual_octopus(skip_transitive_edges: bool) {
     let root_commit = repo.store().root_commit();
 
     let revset = revset_for_commits(repo.as_ref(), &[&commit_a, &commit_b, &commit_c, &commit_f]);
-    let commits = revset.iter_graph_impl(skip_transitive_edges).collect_vec();
+    let commits: Vec<_> = revset
+        .iter_graph_impl(skip_transitive_edges)
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 4);
     assert_eq!(commits[0].0, *commit_f.id());
     assert_eq!(commits[1].0, *commit_c.id());
@@ -162,7 +168,10 @@ fn test_graph_iterator_simple_fork(skip_transitive_edges: bool) {
     let root_commit = repo.store().root_commit();
 
     let revset = revset_for_commits(repo.as_ref(), &[&commit_a, &commit_c, &commit_e]);
-    let commits = revset.iter_graph_impl(skip_transitive_edges).collect_vec();
+    let commits: Vec<_> = revset
+        .iter_graph_impl(skip_transitive_edges)
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 3);
     assert_eq!(commits[0].0, *commit_e.id());
     assert_eq!(commits[1].0, *commit_c.id());
@@ -200,7 +209,10 @@ fn test_graph_iterator_multiple_missing(skip_transitive_edges: bool) {
     let root_commit = repo.store().root_commit();
 
     let revset = revset_for_commits(repo.as_ref(), &[&commit_b, &commit_f]);
-    let commits = revset.iter_graph_impl(skip_transitive_edges).collect_vec();
+    let commits: Vec<_> = revset
+        .iter_graph_impl(skip_transitive_edges)
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 2);
     assert_eq!(commits[0].0, *commit_f.id());
     assert_eq!(commits[1].0, *commit_b.id());
@@ -241,7 +253,10 @@ fn test_graph_iterator_edge_to_ancestor(skip_transitive_edges: bool) {
     let repo = tx.commit("test");
 
     let revset = revset_for_commits(repo.as_ref(), &[&commit_c, &commit_d, &commit_f]);
-    let commits = revset.iter_graph_impl(skip_transitive_edges).collect_vec();
+    let commits: Vec<_> = revset
+        .iter_graph_impl(skip_transitive_edges)
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 3);
     assert_eq!(commits[0].0, *commit_f.id());
     assert_eq!(commits[1].0, *commit_d.id());
@@ -297,7 +312,10 @@ fn test_graph_iterator_edge_escapes_from_(skip_transitive_edges: bool) {
         repo.as_ref(),
         &[&commit_a, &commit_d, &commit_g, &commit_h, &commit_j],
     );
-    let commits = revset.iter_graph_impl(skip_transitive_edges).collect_vec();
+    let commits: Vec<_> = revset
+        .iter_graph_impl(skip_transitive_edges)
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 5);
     assert_eq!(commits[0].0, *commit_j.id());
     assert_eq!(commits[1].0, *commit_h.id());
diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs
index 1fc61b0932..af259af7bb 100644
--- a/lib/tests/test_revset.rs
+++ b/lib/tests/test_revset.rs
@@ -462,6 +462,7 @@ fn test_resolve_working_copy() {
         .evaluate_programmatic(mut_repo)
         .unwrap()
         .iter()
+        .map(Result::unwrap)
         .collect_vec(),
         vec![]
     );
@@ -478,6 +479,7 @@ fn test_resolve_working_copy() {
             .evaluate_programmatic(mut_repo)
             .unwrap()
             .iter()
+            .map(Result::unwrap)
             .collect()
     };
 
@@ -515,6 +517,7 @@ fn test_resolve_working_copies() {
             .evaluate_programmatic(mut_repo)
             .unwrap()
             .iter()
+            .map(Result::unwrap)
             .collect()
     };
 
@@ -968,7 +971,12 @@ fn try_resolve_commit_ids(
     let expression = optimize(parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap());
     let symbol_resolver = DefaultSymbolResolver::new(repo, revset_extensions.symbol_resolvers());
     let expression = expression.resolve_user_expression(repo, &symbol_resolver)?;
-    Ok(expression.evaluate(repo).unwrap().iter().collect())
+    Ok(expression
+        .evaluate(repo)
+        .unwrap()
+        .iter()
+        .map(Result::unwrap)
+        .collect())
 }
 
 fn resolve_commit_ids_in_workspace(
@@ -1001,7 +1009,12 @@ fn resolve_commit_ids_in_workspace(
     let expression = expression
         .resolve_user_expression(repo, &symbol_resolver)
         .unwrap();
-    expression.evaluate(repo).unwrap().iter().collect()
+    expression
+        .evaluate(repo)
+        .unwrap()
+        .iter()
+        .map(Result::unwrap)
+        .collect()
 }
 
 #[test]
@@ -3387,7 +3400,7 @@ fn test_evaluate_expression_file() {
             FilesetExpression::prefix_path(file_path.to_owned()),
         ));
         let revset = expression.evaluate_programmatic(mut_repo).unwrap();
-        revset.iter().collect()
+        revset.iter().map(Result::unwrap).collect()
     };
 
     assert_eq!(resolve(added_clean_clean), vec![commit1.id().clone()]);
@@ -3731,7 +3744,10 @@ fn test_reverse_graph_iterator() {
         repo.as_ref(),
         &[&commit_a, &commit_c, &commit_d, &commit_e, &commit_f],
     );
-    let commits = ReverseGraphIterator::new(revset.iter_graph()).collect_vec();
+    let commits: Vec<_> = ReverseGraphIterator::new(revset.iter_graph())
+        .unwrap()
+        .try_collect()
+        .unwrap();
     assert_eq!(commits.len(), 5);
     assert_eq!(commits[0].0, *commit_a.id());
     assert_eq!(commits[1].0, *commit_c.id());