Skip to content

Commit d16b5d7

Browse files
simolus3simolus3
committed
Improve mismatch errors
1 parent 6801a49 commit d16b5d7

File tree

10 files changed

+87
-44
lines changed

10 files changed

+87
-44
lines changed

‎crates/core/src/bson/error.rs

+37-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use core::fmt::Display;
1+
use core::{fmt::Display, str::Utf8Error};
22

33
use alloc::{
44
boxed::Box,
@@ -27,7 +27,7 @@ pub enum ErrorKind {
2727
Custom(String),
2828
UnknownElementType(i8),
2929
UnterminatedCString,
30-
InvalidCString,
30+
InvalidCString(Utf8Error),
3131
UnexpectedEoF,
3232
InvalidEndOfDocument,
3333
InvalidSize,
@@ -49,7 +49,41 @@ impl BsonError {
4949

5050
impl Display for BsonError {
5151
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
52-
write!(f, "bson error: {:?}", &self.err)
52+
self.err.fmt(f)
53+
}
54+
}
55+
56+
impl Display for BsonErrorImpl {
57+
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
58+
if let Some(offset) = self.offset {
59+
write!(f, "bson error, at {offset}: {}", self.kind)
60+
} else {
61+
write!(f, "bson error at unknown offset: {}", self.kind)
62+
}
63+
}
64+
}
65+
66+
impl Display for ErrorKind {
67+
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
68+
match self {
69+
ErrorKind::Custom(msg) => write!(f, "custom {msg}"),
70+
ErrorKind::UnknownElementType(code) => write!(f, "unknown element code: {code}"),
71+
ErrorKind::UnterminatedCString => write!(f, "unterminated cstring"),
72+
ErrorKind::InvalidCString(e) => write!(f, "cstring with non-utf8 content: {e}"),
73+
ErrorKind::UnexpectedEoF => write!(f, "unexpected end of file"),
74+
ErrorKind::InvalidEndOfDocument => write!(f, "unexpected end of document"),
75+
ErrorKind::InvalidSize => write!(f, "invalid document size"),
76+
ErrorKind::InvalidStateExpectedType => write!(f, "internal state error, expected type"),
77+
ErrorKind::InvalidStateExpectedName => write!(f, "internal state error, expected name"),
78+
ErrorKind::InvalidStateExpectedValue => {
79+
write!(f, "internal state error, expected value")
80+
}
81+
ErrorKind::ExpectedEnum { actual } => write!(f, "expected enum, got {}", *actual as u8),
82+
ErrorKind::ExpectedString => write!(f, "expected a string value"),
83+
ErrorKind::UnexpectedEndOfDocumentForEnumVariant => {
84+
write!(f, "unexpected end of document for enum variant")
85+
}
86+
}
5387
}
5488
}
5589

‎crates/core/src/bson/parser.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl<'de> Parser<'de> {
5050
.map_err(|_| self.error(ErrorKind::UnterminatedCString))?;
5151
let str = raw
5252
.to_str()
53-
.map_err(|_| self.error(ErrorKind::InvalidCString))?;
53+
.map_err(|e| self.error(ErrorKind::InvalidCString(e)))?;
5454

5555
self.advance(str.len() + 1);
5656
Ok(str)
@@ -95,7 +95,7 @@ impl<'de> Parser<'de> {
9595
let bytes = self.advance_checked(length_including_null)?;
9696

9797
str::from_utf8(&bytes[..length_including_null - 1])
98-
.map_err(|_| self.error(ErrorKind::InvalidCString))
98+
.map_err(|e| self.error(ErrorKind::InvalidCString(e)))
9999
}
100100

101101
pub fn read_binary(&mut self) -> Result<(BinarySubtype, &'de [u8]), BsonError> {
@@ -190,7 +190,7 @@ impl<'de> Parser<'de> {
190190
#[repr(transparent)]
191191
pub struct BinarySubtype(pub u8);
192192

193-
#[derive(Clone, Debug)]
193+
#[derive(Clone, Copy, Debug)]
194194
pub enum ElementType {
195195
Double = 1,
196196
String = 2,

‎crates/core/src/sync/checksum.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ impl<'de> Deserialize<'de> for Checksum {
126126
where
127127
E: serde::de::Error,
128128
{
129-
if !v.is_finite() || v.trunc() != v {
129+
if !v.is_finite() || f64::trunc(v) != v {
130130
return Err(E::invalid_value(
131131
serde::de::Unexpected::Float(v),
132132
&"a whole number",

‎crates/core/src/sync/line.rs

+18-17
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,14 @@ pub struct CheckpointDiff<'a> {
6060

6161
#[derive(Deserialize, Debug)]
6262
pub struct CheckpointComplete {
63-
#[serde(deserialize_with = "deserialize_string_to_i64")]
64-
pub last_op_id: i64,
63+
// #[serde(deserialize_with = "deserialize_string_to_i64")]
64+
// pub last_op_id: i64,
6565
}
6666

6767
#[derive(Deserialize, Debug)]
6868
pub struct CheckpointPartiallyComplete {
69-
#[serde(deserialize_with = "deserialize_string_to_i64")]
70-
pub last_op_id: i64,
69+
// #[serde(deserialize_with = "deserialize_string_to_i64")]
70+
// pub last_op_id: i64,
7171
pub priority: BucketPriority,
7272
}
7373

@@ -77,21 +77,21 @@ pub struct BucketChecksum<'a> {
7777
pub checksum: Checksum,
7878
pub priority: Option<BucketPriority>,
7979
pub count: Option<i64>,
80-
#[serde(default)]
81-
#[serde(deserialize_with = "deserialize_optional_string_to_i64")]
82-
pub last_op_id: Option<i64>,
80+
// #[serde(default)]
81+
// #[serde(deserialize_with = "deserialize_optional_string_to_i64")]
82+
// pub last_op_id: Option<i64>,
8383
}
8484

8585
#[derive(Deserialize, Debug)]
8686
pub struct DataLine<'a> {
8787
pub bucket: &'a str,
8888
pub data: Vec<OplogEntry<'a>>,
89-
#[serde(default)]
90-
pub has_more: bool,
91-
#[serde(default, borrow)]
92-
pub after: Option<&'a str>,
93-
#[serde(default, borrow)]
94-
pub next_after: Option<&'a str>,
89+
// #[serde(default)]
90+
// pub has_more: bool,
91+
// #[serde(default, borrow)]
92+
// pub after: Option<&'a str>,
93+
// #[serde(default, borrow)]
94+
// pub next_after: Option<&'a str>,
9595
}
9696

9797
#[derive(Deserialize, Debug)]
@@ -368,16 +368,19 @@ mod tests {
368368
fn parse_checkpoint_complete() {
369369
assert_matches!(
370370
deserialize(r#"{"checkpoint_complete": {"last_op_id": "10"}}"#),
371-
SyncLine::CheckpointComplete(CheckpointComplete { last_op_id: 10 })
371+
SyncLine::CheckpointComplete(CheckpointComplete {
372+
//last_op_id: 10
373+
})
372374
);
375+
373376
}
374377

375378
#[test]
376379
fn parse_checkpoint_partially_complete() {
377380
assert_matches!(
378381
deserialize(r#"{"partial_checkpoint_complete": {"last_op_id": "10", "priority": 1}}"#),
379382
SyncLine::CheckpointPartiallyComplete(CheckpointPartiallyComplete {
380-
last_op_id: 10,
383+
//last_op_id: 10,
381384
priority: BucketPriority { number: 1 }
382385
})
383386
);
@@ -397,8 +400,6 @@ mod tests {
397400
};
398401

399402
assert_eq!(data.bucket, "bkt");
400-
assert_eq!(data.after, None);
401-
assert_eq!(data.next_after, None);
402403

403404
assert_eq!(data.data.len(), 1);
404405
let entry = &data.data[0];

‎crates/core/src/sync/storage_adapter.rs

-5
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,3 @@ pub struct PersistedBucketProgress<'a> {
356356
pub count_at_last: i64,
357357
pub count_since_last: i64,
358358
}
359-
360-
pub struct BucketDescription {
361-
pub priority: BucketPriority,
362-
pub name: String,
363-
}

‎crates/core/src/sync/streaming_sync.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ impl StreamingSyncIteration {
335335
severity: LogSeverity::DEBUG,
336336
line: "Validated and applied checkpoint".into(),
337337
});
338+
event.instructions.push(Instruction::FlushFileSystem {});
338339
self.handle_checkpoint_applied(event)?;
339340
}
340341
}
@@ -372,6 +373,7 @@ impl StreamingSyncIteration {
372373
}
373374
SyncLocalResult::ChangesApplied => {
374375
let now = self.adapter.now()?;
376+
event.instructions.push(Instruction::FlushFileSystem {});
375377
self.status.update(
376378
|status| {
377379
status.partial_checkpoint_complete(priority, now);
@@ -539,7 +541,6 @@ pub struct OwnedBucketChecksum {
539541
pub checksum: Checksum,
540542
pub priority: BucketPriority,
541543
pub count: Option<i64>,
542-
pub last_op_id: Option<i64>,
543544
}
544545

545546
impl OwnedBucketChecksum {
@@ -558,7 +559,6 @@ impl From<&'_ BucketChecksum<'_>> for OwnedBucketChecksum {
558559
checksum: value.checksum,
559560
priority: value.priority.unwrap_or(BucketPriority::FALLBACK),
560561
count: value.count,
561-
last_op_id: value.last_op_id,
562562
}
563563
}
564564
}

‎dart/test/goldens/simple_iteration.json

+3
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@
123123
"line": "Validated and applied checkpoint"
124124
}
125125
},
126+
{
127+
"FlushFileSystem": {}
128+
},
126129
{
127130
"DidCompleteSync": {}
128131
},

‎dart/test/legacy_sync_test.dart

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import 'utils/native_test_utils.dart';
1212
/// Tests that the older sync interfaces requiring clients to decode and handle
1313
/// sync lines still work.
1414
void main() {
15-
final vfs = TestSqliteFileSystem(fs: const LocalFileSystem());
15+
final vfs = TestSqliteFileSystem(
16+
fs: const LocalFileSystem(), name: 'legacy-sync-test');
1617

1718
setUpAll(() {
1819
loadExtension();

‎dart/test/sync_test.dart

+20-12
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ void syncTest(String description, void Function(FakeAsync controller) body) {
2323
}
2424

2525
void main() {
26-
final vfs = TestSqliteFileSystem(fs: const LocalFileSystem());
26+
final vfs =
27+
TestSqliteFileSystem(fs: const LocalFileSystem(), name: 'vfs-sync-test');
2728

2829
setUpAll(() {
2930
loadExtension();
@@ -728,8 +729,14 @@ final class SyncLinesGoldenTest {
728729
return expectedLines[actualLines.length];
729730
}
730731

731-
Never _mismatch() {
732-
throw 'Golden test for sync lines failed, set UPDATE_GOLDENS=1 to update';
732+
void _checkMismatch(void Function() compare) {
733+
try {
734+
compare();
735+
} catch (e) {
736+
print(
737+
'Golden test for sync lines failed, set UPDATE_GOLDENS=1 to update');
738+
rethrow;
739+
}
733740
}
734741

735742
void load(String name) {
@@ -762,16 +769,16 @@ final class SyncLinesGoldenTest {
762769
if (!isBson) {
763770
// We only want to compare the JSON inputs. We compare outputs
764771
// regardless of the encoding mode.
765-
if (expected.operation != operation ||
766-
json.encode(expected.data) != json.encode(matchData)) {
767-
_mismatch();
768-
}
772+
_checkMismatch(() {
773+
expect(operation, expected.operation);
774+
expect(matchData, expected.data);
775+
});
769776
}
770777

771778
final result = _invokeControl(operation, data);
772-
if (json.encode(result) != json.encode(expected.output)) {
773-
_mismatch();
774-
}
779+
_checkMismatch(() {
780+
expect(result, expected.output);
781+
});
775782

776783
actualLines.add(ExpectedSyncLine(operation, matchData, result));
777784
return result;
@@ -784,8 +791,9 @@ final class SyncLinesGoldenTest {
784791
File(path).writeAsStringSync(
785792
JsonEncoder.withIndent(' ').convert(actualLines));
786793
}
787-
} else if (expectedLines.length != actualLines.length) {
788-
_mismatch();
794+
} else {
795+
_checkMismatch(
796+
() => expect(actualLines, hasLength(expectedLines.length)));
789797
}
790798
}
791799
}

‎tool/build_wasm.sh

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/bin/bash
22
set -e
3+
emcc --version
34

45
# Normal build
56
# target/wasm32-unknown-emscripten/wasm/powersync.wasm

0 commit comments

Comments
 (0)