Skip to content

Commit 4ccf048

Browse files
zxch3nzxch3n
authored
fix: propagate errors instead of unwrap (#808)
* fix: propagate errors instead of unwrap - Replace unwrap() with ? in attach branches: - TextHandler::attach (apply_delta_with_txn) - MapHandler::attach (insert_with_txn, insert_container_with_txn) - MovableListHandler::attach (insert_with_txn, insert_container_with_txn) - ListHandler::attach (insert_with_txn, insert_container_with_txn) - Prevents panics and returns errors to the caller - Build succeeds * chore: changeset
1 parent 88d9785 commit 4ccf048

File tree

5 files changed

+83
-63
lines changed

5 files changed

+83
-63
lines changed

‎.changeset/witty-clouds-cross.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"loro-crdt": patch
3+
---
4+
5+
fix: propagate errors instead of unwrap #808

‎crates/loro-internal/src/handler.rs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ impl HandlerTrait for TextHandler {
289289
let ans = new_inner.into_text().unwrap();
290290

291291
let delta = self.get_delta();
292-
ans.apply_delta_with_txn(txn, &delta).unwrap();
292+
ans.apply_delta_with_txn(txn, &delta)?;
293293
Ok(ans)
294294
}
295295
}
@@ -539,10 +539,9 @@ impl HandlerTrait for MapHandler {
539539

540540
for (k, v) in self.get_value().into_map().unwrap().iter() {
541541
if let LoroValue::Container(id) = v {
542-
ans.insert_container_with_txn(txn, k, create_handler(a, id.clone()))
543-
.unwrap();
542+
ans.insert_container_with_txn(txn, k, create_handler(a, id.clone()))?;
544543
} else {
545-
ans.insert_with_txn(txn, k, v.clone()).unwrap();
544+
ans.insert_with_txn(txn, k, v.clone())?;
546545
}
547546
}
548547

@@ -671,10 +670,9 @@ impl HandlerTrait for MovableListHandler {
671670

672671
for (i, v) in self.get_value().into_list().unwrap().iter().enumerate() {
673672
if let LoroValue::Container(id) = v {
674-
ans.insert_container_with_txn(txn, i, create_handler(a, id.clone()))
675-
.unwrap();
673+
ans.insert_container_with_txn(txn, i, create_handler(a, id.clone()))?;
676674
} else {
677-
ans.insert_with_txn(txn, i, v.clone()).unwrap();
675+
ans.insert_with_txn(txn, i, v.clone())?;
678676
}
679677
}
680678

@@ -782,10 +780,9 @@ impl HandlerTrait for ListHandler {
782780

783781
for (i, v) in self.get_value().into_list().unwrap().iter().enumerate() {
784782
if let LoroValue::Container(id) = v {
785-
ans.insert_container_with_txn(txn, i, create_handler(a, id.clone()))
786-
.unwrap();
783+
ans.insert_container_with_txn(txn, i, create_handler(a, id.clone()))?;
787784
} else {
788-
ans.insert_with_txn(txn, i, v.clone()).unwrap();
785+
ans.insert_with_txn(txn, i, v.clone())?;
789786
}
790787
}
791788

‎crates/loro-internal/src/handler/movable_list_apply_delta.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,7 @@ impl MovableListHandler {
122122
}
123123

124124
// Apply any remaining deletions.
125-
self.apply_remaining_deletions(delta_change, &mut deleted_indices)
126-
.unwrap();
127-
125+
self.apply_remaining_deletions(delta_change, &mut deleted_indices)?;
128126
Ok(())
129127
}
130128
}

‎crates/loro-wasm/VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.7.0
1+
1.7.1

‎crates/loro-wasm/src/lib.rs‎

Lines changed: 69 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -504,21 +504,27 @@ impl LoroDoc {
504504
let mut style_config = StyleConfigMap::new();
505505
// read key value pair in styles
506506
for key in Reflect::own_keys(&styles)?.iter() {
507-
let value = Reflect::get(&styles, &key).unwrap();
508-
let key = key.as_string().unwrap();
507+
let value = Reflect::get(&styles, &key)?;
508+
let key = key
509+
.as_string()
510+
.ok_or_else(|| JsError::new("Text style key must be a string"))?;
509511
// Assert value is an object, otherwise throw an error with desc
510512
if !value.is_object() {
511513
return Err("Text style config format error".into());
512514
}
513515
// read expand value from value
514-
let expand = Reflect::get(&value, &"expand".into()).expect("`expand` not specified");
515-
let expand_str = expand.as_string().unwrap();
516+
let expand = Reflect::get(&value, &"expand".into())
517+
.map_err(|_| JsError::new("`expand` not specified"))?;
518+
let expand_str = expand
519+
.as_string()
520+
.ok_or_else(|| JsError::new("`expand` must be a string"))?;
516521
// read allowOverlap value from value
517522
style_config.insert(
518523
key.into(),
519524
StyleConfig {
520-
expand: ExpandType::try_from_str(&expand_str)
521-
.expect("`expand` must be one of `none`, `start`, `end`, `both`"),
525+
expand: ExpandType::try_from_str(&expand_str).ok_or_else(|| {
526+
JsError::new("`expand` must be one of `none`, `start`, `end`, `both`")
527+
})?,
522528
},
523529
);
524530
}
@@ -538,11 +544,15 @@ impl LoroDoc {
538544
self.0.config_default_text_style(None);
539545
} else {
540546
let value = style.obj;
541-
let expand = Reflect::get(&value, &"expand".into()).expect("`expand` not specified");
542-
let expand_str = expand.as_string().unwrap();
547+
let expand = Reflect::get(&value, &"expand".into())
548+
.map_err(|_| JsError::new("`expand` not specified"))?;
549+
let expand_str = expand
550+
.as_string()
551+
.ok_or_else(|| JsError::new("`expand` must be a string"))?;
543552

544-
style_config.expand = ExpandType::try_from_str(&expand_str)
545-
.expect("`expand` must be one of `none`, `start`, `end`, `both`");
553+
style_config.expand = ExpandType::try_from_str(&expand_str).ok_or_else(|| {
554+
JsError::new("`expand` must be one of `none`, `start`, `end`, `both`")
555+
})?;
546556

547557
self.0.config_default_text_style(Some(style_config));
548558
}
@@ -700,40 +710,38 @@ impl LoroDoc {
700710
Err(_) => return Err(JsValue::from_str("Expected a function")),
701711
};
702712
let observer = observer::Observer::new(f);
713+
let ids: Result<Vec<_>, _> = ids.into_iter().map(js_id_to_id).collect();
714+
let ids = ids?;
703715
self.0
704-
.travel_change_ancestors(
705-
&ids.into_iter()
706-
.map(|id| js_id_to_id(id).unwrap())
707-
.collect::<Vec<_>>(),
708-
&mut |meta| {
709-
let res = observer
710-
.call1(
711-
&ChangeMeta {
712-
lamport: meta.lamport,
713-
length: meta.len as u32,
714-
peer: meta.id.peer.to_string(),
715-
counter: meta.id.counter,
716-
deps: meta
717-
.deps
718-
.iter()
719-
.map(|id| StringID {
720-
peer: id.peer.to_string(),
721-
counter: id.counter,
722-
})
723-
.collect(),
724-
timestamp: meta.timestamp as f64,
725-
message: meta.message,
726-
}
727-
.to_js(),
728-
)
729-
.unwrap();
730-
if res.as_bool().unwrap() {
731-
ControlFlow::Continue(())
732-
} else {
733-
ControlFlow::Break(())
716+
.travel_change_ancestors(&ids, &mut |meta| {
717+
let res = match observer.call1(
718+
&ChangeMeta {
719+
lamport: meta.lamport,
720+
length: meta.len as u32,
721+
peer: meta.id.peer.to_string(),
722+
counter: meta.id.counter,
723+
deps: meta
724+
.deps
725+
.iter()
726+
.map(|id| StringID {
727+
peer: id.peer.to_string(),
728+
counter: id.counter,
729+
})
730+
.collect(),
731+
timestamp: meta.timestamp as f64,
732+
message: meta.message,
734733
}
735-
},
736-
)
734+
.to_js(),
735+
) {
736+
Ok(v) => v,
737+
Err(_) => return ControlFlow::Continue(()),
738+
};
739+
if res.as_bool().unwrap_or(true) {
740+
ControlFlow::Continue(())
741+
} else {
742+
ControlFlow::Break(())
743+
}
744+
})
737745
.map_err(|e| JsValue::from(e.to_string()))
738746
}
739747

@@ -1814,8 +1822,8 @@ impl LoroDoc {
18141822
let ops = change
18151823
.ops()
18161824
.iter()
1817-
.map(|op| op.serialize(&serializer).unwrap())
1818-
.collect::<Vec<_>>();
1825+
.map(|op| op.serialize(&serializer))
1826+
.collect::<Result<Vec<_>, _>>()?;
18191827

18201828
Ok(ops)
18211829
}
@@ -2361,7 +2369,10 @@ impl LoroText {
23612369
};
23622370
let context = JsValue::NULL;
23632371
self.handler.iter(|c| {
2364-
let result = callback.call1(&context, &JsValue::from(c)).unwrap();
2372+
let result = match callback.call1(&context, &JsValue::from(c)) {
2373+
Ok(v) => v,
2374+
Err(_) => return true,
2375+
};
23652376
match result.as_bool() {
23662377
Some(true) => true,
23672378
Some(false) => false,
@@ -4126,7 +4137,10 @@ impl LoroTreeNode {
41264137
.get_all_hierarchy_nodes_under(TreeParentId::Node(self.id));
41274138
let node = TreeNodeWithChildren {
41284139
id: self.id,
4129-
parent: self.tree.get_node_parent(&self.id).unwrap(),
4140+
parent: self
4141+
.tree
4142+
.get_node_parent(&self.id)
4143+
.ok_or_else(|| JsError::new("Tree node parent not found"))?,
41304144
fractional_index: self
41314145
.tree
41324146
.get_position_by_tree_id(&self.id)
@@ -4323,14 +4337,17 @@ impl LoroTree {
43234337
#[wasm_bindgen(js_name = "has")]
43244338
pub fn contains(&self, target: &JsTreeID) -> bool {
43254339
let target: JsValue = target.into();
4326-
self.handler.contains(target.try_into().unwrap())
4340+
match TreeID::try_from(target) {
4341+
Ok(id) => self.handler.contains(id),
4342+
Err(_) => false,
4343+
}
43274344
}
43284345

43294346
/// Return `None` if the node is not exist, otherwise return `Some(true)` if the node is deleted.
43304347
#[wasm_bindgen(js_name = "isNodeDeleted")]
43314348
pub fn is_node_deleted(&self, target: &JsTreeID) -> JsResult<bool> {
43324349
let target: JsValue = target.into();
4333-
let target = TreeID::try_from(target).unwrap();
4350+
let target = TreeID::try_from(target)?;
43344351
let ans = self.handler.is_node_deleted(&target)?;
43354352
Ok(ans)
43364353
}
@@ -4398,7 +4415,10 @@ impl LoroTree {
43984415
.unwrap_or(JsValue::undefined());
43994416
let index = v.index;
44004417
let position = v.fractional_index.to_string();
4401-
let map: LoroMap = self.get_node_by_id(&id).unwrap().data()?;
4418+
let map: LoroMap = self
4419+
.get_node_by_id(&id)
4420+
.ok_or_else(|| JsError::new("Tree node not found"))?
4421+
.data()?;
44024422
let obj = Object::new();
44034423
js_sys::Reflect::set(&obj, &"id".into(), &id)?;
44044424
js_sys::Reflect::set(&obj, &"parent".into(), &parent)?;

0 commit comments

Comments
 (0)