Skip to content

Commit 145e5d6

Browse files
feat: propagate persistence backend errors via Result
The Persistence contract previously returned `Future[void]` for writes and `Future[ChannelSnapshot]` for the loader, with `raises: []`. Backends had no way to report a failure, so a failed write or a failed/partial read was silently swallowed — and on the read path a mid-scan failure could bootstrap a *truncated* channel snapshot, corrupting the rebuilt bloom filter and lamport clock across a restart. Make every contract field Result-returning: * mutating ops -> Future[Result[void, string]] * loadAllForChannel -> Future[Result[ChannelSnapshot, string]] The backend-supplied error string is mapped to a new `ReliabilityError.rePersistenceError` (logged once at the boundary via `reliabilityErr`) and threaded up through every persistence-touching proc to the public API, where the caller decides what to do. Request-driven paths (wrap/unwrap/markDependenciesMet/ensureChannel/removeChannel/reset) propagate the error; background maintenance loops (periodicBufferSweep, periodicRepairSweep) log and retry on the next tick, since they have no synchronous caller. Tests: in-memory backend gains a `failingOps` injection hook; new "Persistence: error propagation" suite asserts read/write/drop failures surface as `rePersistenceError`. Full suite passes (90 OK). BREAKING CHANGE: the `Persistence` contract signature changed; custom backends must return `Result` and `ok()` on success. Bumped to 0.3.0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 980c830 commit 145e5d6

8 files changed

Lines changed: 353 additions & 181 deletions

File tree

sds.nim

Lines changed: 116 additions & 53 deletions
Large diffs are not rendered by default.

sds.nimble

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import strutils, os
22

33
# Package
4-
version = "0.2.4"
4+
version = "0.3.0"
55
author = "Logos Messaging Team"
66
description = "E2E Scalable Data Sync API"
77
license = "MIT"

sds/sds_utils.nim

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,24 @@ export
1515
proc defaultConfig*(): ReliabilityConfig =
1616
return ReliabilityConfig.init()
1717

18+
proc reliabilityErr*(detail: string): ReliabilityError {.gcsafe, raises: [].} =
19+
## Maps a backend-supplied persistence error string onto the
20+
## `rePersistenceError` enum value. The enum carries no payload, so the
21+
## original detail is logged here — this is the single point where a
22+
## persistence failure is recorded, while the enum value travels up the
23+
## `Result` chain to the public API caller, who decides what to do.
24+
warn "persistence operation failed", detail = detail
25+
ReliabilityError.rePersistenceError
26+
1827
proc dropChannelFromPersistence*(
1928
rm: ReliabilityManager, channelId: SdsChannelID
20-
) {.async: (raises: []).} =
29+
): Future[Result[void, ReliabilityError]] {.async: (raises: []).} =
2130
## Wipes all persisted state for a channel via a single backend call.
2231
## Called by removeChannel / resetReliabilityManager before they clear
2332
## in-memory state. Backend executes the wipe in one transaction.
24-
await rm.persistence.dropChannel(channelId)
33+
(await rm.persistence.dropChannel(channelId)).isOkOr:
34+
return err(reliabilityErr(error))
35+
ok()
2536

2637
proc cleanup*(rm: ReliabilityManager) {.async: (raises: []).} =
2738
## Releases in-memory state. Does NOT wipe persistence — the manager may be
@@ -69,7 +80,7 @@ proc cleanBloomFilter*(
6980

7081
proc addToHistory*(
7182
rm: ReliabilityManager, msg: SdsMessage, channelId: SdsChannelID
72-
) {.async: (raises: []).} =
83+
): Future[Result[void, ReliabilityError]] {.async: (raises: []).} =
7384
## Inserts a delivered message into the channel's history map and evicts the
7485
## eldest entries when the bound is exceeded. The full SdsMessage is kept so
7586
## senderId is available for downstream causal-history population and the
@@ -78,29 +89,36 @@ proc addToHistory*(
7889
if channelId in rm.channels:
7990
let channel = rm.channels[channelId]
8091
channel.messageHistory[msg.messageId] = msg
81-
await rm.persistence.appendLogEntry(channelId, msg)
92+
(await rm.persistence.appendLogEntry(channelId, msg)).isOkOr:
93+
return err(reliabilityErr(error))
8294
while channel.messageHistory.len > rm.config.maxMessageHistory:
8395
var firstKey: SdsMessageID
8496
for k in channel.messageHistory.keys:
8597
firstKey = k
8698
break
8799
channel.messageHistory.del(firstKey)
88-
await rm.persistence.removeLogEntry(channelId, firstKey)
100+
(await rm.persistence.removeLogEntry(channelId, firstKey)).isOkOr:
101+
return err(reliabilityErr(error))
102+
ok()
89103
except CatchableError:
90104
error "Failed to add to history",
91105
channelId = channelId, msgId = msg.messageId, error = getCurrentExceptionMsg()
106+
err(ReliabilityError.reInternalError)
92107

93108
proc updateLamportTimestamp*(
94109
rm: ReliabilityManager, msgTs: int64, channelId: SdsChannelID
95-
) {.async: (raises: []).} =
110+
): Future[Result[void, ReliabilityError]] {.async: (raises: []).} =
96111
try:
97112
if channelId in rm.channels:
98113
let channel = rm.channels[channelId]
99114
channel.lamportTimestamp = max(msgTs, channel.lamportTimestamp) + 1
100-
await rm.persistence.saveLamport(channelId, channel.lamportTimestamp)
115+
(await rm.persistence.saveLamport(channelId, channel.lamportTimestamp)).isOkOr:
116+
return err(reliabilityErr(error))
117+
ok()
101118
except CatchableError:
102119
error "Failed to update lamport timestamp",
103120
channelId = channelId, msgTs = msgTs, error = getCurrentExceptionMsg()
121+
err(ReliabilityError.reInternalError)
104122

105123
proc newHistoryEntry*(
106124
messageId: SdsMessageID, retrievalHint: seq[byte] = @[]
@@ -167,7 +185,7 @@ proc isInResponseGroup*(
167185

168186
proc getRecentHistoryEntries*(
169187
rm: ReliabilityManager, n: int, channelId: SdsChannelID
170-
): Future[seq[HistoryEntry]] {.async: (raises: []).} =
188+
): Future[Result[seq[HistoryEntry], ReliabilityError]] {.async: (raises: []).} =
171189
## Get recent history entries for sending in causal history.
172190
## Populates retrieval hints and senderId (SDS-R) for each entry.
173191
try:
@@ -184,16 +202,17 @@ proc getRecentHistoryEntries*(
184202
{.cast(raises: []).}:
185203
entry.retrievalHint = rm.onRetrievalHint(msgId)
186204
if entry.retrievalHint.len > 0:
187-
await rm.persistence.setRetrievalHint(msgId, entry.retrievalHint)
205+
(await rm.persistence.setRetrievalHint(msgId, entry.retrievalHint)).isOkOr:
206+
return err(reliabilityErr(error))
188207
entry.senderId = channel.messageHistory[msgId].senderId
189208
entries.add(entry)
190-
return entries
209+
ok(entries)
191210
else:
192-
return @[]
211+
ok(newSeq[HistoryEntry]())
193212
except CatchableError:
194213
error "Failed to get recent history entries",
195214
channelId = channelId, n = n, error = getCurrentExceptionMsg()
196-
return @[]
215+
err(ReliabilityError.reInternalError)
197216

198217
proc checkDependencies*(
199218
rm: ReliabilityManager, deps: seq[HistoryEntry], channelId: SdsChannelID
@@ -269,7 +288,7 @@ proc getIncomingBuffer*(
269288

270289
proc getOrCreateChannel*(
271290
rm: ReliabilityManager, channelId: SdsChannelID
272-
): Future[ChannelContext] {.async: (raises: [CatchableError]).} =
291+
): Future[Result[ChannelContext, ReliabilityError]] {.async: (raises: []).} =
273292
## Returns the channel context, creating and bootstrapping it from the
274293
## persistence backend if it does not yet exist in memory. The bloom filter
275294
## is rebuilt deterministically from the loaded message history rather than
@@ -281,7 +300,8 @@ proc getOrCreateChannel*(
281300
rm.config.bloomFilterCapacity, rm.config.bloomFilterErrorRate
282301
)
283302
)
284-
let snapshot = await rm.persistence.loadAllForChannel(channelId)
303+
let snapshot = (await rm.persistence.loadAllForChannel(channelId)).valueOr:
304+
return err(reliabilityErr(error))
285305
channel.lamportTimestamp = snapshot.lamportTimestamp
286306
for msg in snapshot.messageHistory:
287307
channel.messageHistory[msg.messageId] = msg
@@ -295,25 +315,21 @@ proc getOrCreateChannel*(
295315
for (msgId, entry) in snapshot.incomingRepairBuffer:
296316
channel.incomingRepairBuffer[msgId] = entry
297317
rm.channels[channelId] = channel
298-
return rm.channels[channelId]
299-
except CatchableError as e:
318+
ok(rm.channels[channelId])
319+
except CatchableError:
300320
error "Failed to get or create channel",
301321
channelId = channelId, error = getCurrentExceptionMsg()
302-
raise e
322+
err(ReliabilityError.reInternalError)
303323

304324
proc ensureChannel*(
305325
rm: ReliabilityManager, channelId: SdsChannelID
306326
): Future[Result[void, ReliabilityError]] {.async: (raises: []).} =
307327
try:
308328
await rm.lock.acquire()
309329
try:
310-
try:
311-
discard await rm.getOrCreateChannel(channelId)
312-
return ok()
313-
except CatchableError:
314-
error "Failed to ensure channel",
315-
channelId = channelId, msg = getCurrentExceptionMsg()
316-
return err(ReliabilityError.reInternalError)
330+
(await rm.getOrCreateChannel(channelId)).isOkOr:
331+
return err(error)
332+
return ok()
317333
finally:
318334
rm.lock.release()
319335
except CatchableError:
@@ -330,7 +346,8 @@ proc removeChannel*(
330346
try:
331347
if channelId in rm.channels:
332348
let channel = rm.channels[channelId]
333-
await rm.dropChannelFromPersistence(channelId)
349+
(await rm.dropChannelFromPersistence(channelId)).isOkOr:
350+
return err(error)
334351
channel.outgoingBuffer.setLen(0)
335352
channel.incomingBuffer.clear()
336353
channel.messageHistory.clear()

sds/types/persistence.nim

Lines changed: 75 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
import chronos
1+
import chronos, results
22
import ./sds_message_id
33
import ./sds_message
44
import ./unacknowledged_message
55
import ./incoming_message
66
import ./repair_entry
77
export
8-
sds_message_id, sds_message, unacknowledged_message, incoming_message, repair_entry
8+
results, sds_message_id, sds_message, unacknowledged_message, incoming_message,
9+
repair_entry
910

1011
## SDS state persistence interface (issue #64).
1112
##
@@ -18,6 +19,14 @@ export
1819
## All proc fields are async (return `Future`) so backends can do real I/O
1920
## without blocking the Chronos event loop the manager runs on.
2021
##
22+
## Every field returns a `Result` so backend failures are propagated to nim-sds
23+
## rather than swallowed by the backend. Mutating ops return
24+
## `Result[void, string]`; the getter (`loadAllForChannel`) returns
25+
## `Result[ChannelSnapshot, string]`. The error is a backend-supplied message;
26+
## nim-sds maps it to `ReliabilityError.rePersistenceError` and surfaces it on
27+
## the corresponding public API call. The contract still forbids raising
28+
## (`raises: []`): failure must travel through the `Result`, not an exception.
29+
##
2130
## Bloom filter is intentionally not persisted: it is rebuilt from the local
2231
## history log on bootstrap. Async timers are likewise recomputed from the
2332
## absolute timestamps stored in the repair buffer entries.
@@ -44,120 +53,125 @@ type
4453
## in-memory state in lockstep without blocking the event loop.
4554

4655
# Per-channel lamport clock
47-
saveLamport*: proc(channelId: SdsChannelID, lamport: int64): Future[void] {.
48-
async: (raises: []), gcsafe
49-
.}
56+
saveLamport*: proc(
57+
channelId: SdsChannelID, lamport: int64
58+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
5059

5160
# Local log (delivered messages)
52-
appendLogEntry*: proc(channelId: SdsChannelID, msg: SdsMessage): Future[void] {.
53-
async: (raises: []), gcsafe
54-
.}
55-
removeLogEntry*: proc(channelId: SdsChannelID, msgId: SdsMessageID): Future[void] {.
56-
async: (raises: []), gcsafe
57-
.}
58-
setRetrievalHint*: proc(msgId: SdsMessageID, hint: seq[byte]): Future[void] {.
59-
async: (raises: []), gcsafe
60-
.}
61+
appendLogEntry*: proc(
62+
channelId: SdsChannelID, msg: SdsMessage
63+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
64+
removeLogEntry*: proc(
65+
channelId: SdsChannelID, msgId: SdsMessageID
66+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
67+
setRetrievalHint*: proc(
68+
msgId: SdsMessageID, hint: seq[byte]
69+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
6170

6271
# Outgoing unacknowledged buffer
6372
saveOutgoing*: proc(
6473
channelId: SdsChannelID, msg: UnacknowledgedMessage
65-
): Future[void] {.async: (raises: []), gcsafe.}
66-
removeOutgoing*: proc(channelId: SdsChannelID, msgId: SdsMessageID): Future[void] {.
67-
async: (raises: []), gcsafe
68-
.}
74+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
75+
removeOutgoing*: proc(
76+
channelId: SdsChannelID, msgId: SdsMessageID
77+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
6978

7079
# Incoming dependency-waiting buffer
71-
saveIncoming*: proc(channelId: SdsChannelID, msg: IncomingMessage): Future[void] {.
72-
async: (raises: []), gcsafe
73-
.}
74-
removeIncoming*: proc(channelId: SdsChannelID, msgId: SdsMessageID): Future[void] {.
75-
async: (raises: []), gcsafe
76-
.}
80+
saveIncoming*: proc(
81+
channelId: SdsChannelID, msg: IncomingMessage
82+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
83+
removeIncoming*: proc(
84+
channelId: SdsChannelID, msgId: SdsMessageID
85+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
7786

7887
# SDS-R outgoing repair buffer
7988
saveOutgoingRepair*: proc(
8089
channelId: SdsChannelID, msgId: SdsMessageID, entry: OutgoingRepairEntry
81-
) {.async: (raises: []).}
90+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
8291
removeOutgoingRepair*: proc(
8392
channelId: SdsChannelID, msgId: SdsMessageID
84-
): Future[void] {.async: (raises: []), gcsafe.}
93+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
8594

8695
# SDS-R incoming repair buffer
8796
saveIncomingRepair*: proc(
8897
channelId: SdsChannelID, msgId: SdsMessageID, entry: IncomingRepairEntry
89-
) {.async: (raises: []).}
98+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
9099
removeIncomingRepair*: proc(
91100
channelId: SdsChannelID, msgId: SdsMessageID
92-
): Future[void] {.async: (raises: []), gcsafe.}
101+
): Future[Result[void, string]] {.async: (raises: []), gcsafe.}
93102

94103
# Wipe all persisted state for a channel in one transactional call.
95104
# Called by removeChannel / resetReliabilityManager. Backends should
96105
# implement this atomically (e.g. one BEGIN/COMMIT) — a per-row loop on
97106
# the nim-sds side would mean N fsyncs per drop.
98-
dropChannel*:
99-
proc(channelId: SdsChannelID): Future[void] {.async: (raises: []), gcsafe.}
100-
101-
# Bootstrap on `addChannel` / `getOrCreateChannel`.
102-
loadAllForChannel*: proc(channelId: SdsChannelID): Future[ChannelSnapshot] {.
107+
dropChannel*: proc(channelId: SdsChannelID): Future[Result[void, string]] {.
103108
async: (raises: []), gcsafe
104109
.}
105110

111+
# Bootstrap on `addChannel` / `getOrCreateChannel`.
112+
loadAllForChannel*: proc(
113+
channelId: SdsChannelID
114+
): Future[Result[ChannelSnapshot, string]] {.async: (raises: []), gcsafe.}
115+
106116
proc noOpPersistence*(): Persistence =
107117
## Default backend that discards every write and returns an empty snapshot.
108118
## Used so existing callers (and tests) that don't care about durability
109119
## keep working without supplying a real backend.
110120
Persistence(
111-
saveLamport: proc(channelId: SdsChannelID, lamport: int64) {.async: (raises: []).} =
112-
discard,
121+
saveLamport: proc(
122+
channelId: SdsChannelID, lamport: int64
123+
): Future[Result[void, string]] {.async: (raises: []).} =
124+
ok(),
113125
appendLogEntry: proc(
114126
channelId: SdsChannelID, msg: SdsMessage
115-
) {.async: (raises: []).} =
116-
discard,
127+
): Future[Result[void, string]] {.async: (raises: []).} =
128+
ok(),
117129
removeLogEntry: proc(
118130
channelId: SdsChannelID, msgId: SdsMessageID
119-
) {.async: (raises: []).} =
120-
discard,
131+
): Future[Result[void, string]] {.async: (raises: []).} =
132+
ok(),
121133
setRetrievalHint: proc(
122134
msgId: SdsMessageID, hint: seq[byte]
123-
) {.async: (raises: []).} =
124-
discard,
135+
): Future[Result[void, string]] {.async: (raises: []).} =
136+
ok(),
125137
saveOutgoing: proc(
126138
channelId: SdsChannelID, msg: UnacknowledgedMessage
127-
) {.async: (raises: []).} =
128-
discard,
139+
): Future[Result[void, string]] {.async: (raises: []).} =
140+
ok(),
129141
removeOutgoing: proc(
130142
channelId: SdsChannelID, msgId: SdsMessageID
131-
) {.async: (raises: []).} =
132-
discard,
143+
): Future[Result[void, string]] {.async: (raises: []).} =
144+
ok(),
133145
saveIncoming: proc(
134146
channelId: SdsChannelID, msg: IncomingMessage
135-
) {.async: (raises: []).} =
136-
discard,
147+
): Future[Result[void, string]] {.async: (raises: []).} =
148+
ok(),
137149
removeIncoming: proc(
138150
channelId: SdsChannelID, msgId: SdsMessageID
139-
) {.async: (raises: []).} =
140-
discard,
151+
): Future[Result[void, string]] {.async: (raises: []).} =
152+
ok(),
141153
saveOutgoingRepair: proc(
142154
channelId: SdsChannelID, msgId: SdsMessageID, entry: OutgoingRepairEntry
143-
) {.async: (raises: []).} =
144-
discard,
155+
): Future[Result[void, string]] {.async: (raises: []).} =
156+
ok(),
145157
removeOutgoingRepair: proc(
146158
channelId: SdsChannelID, msgId: SdsMessageID
147-
) {.async: (raises: []).} =
148-
discard,
159+
): Future[Result[void, string]] {.async: (raises: []).} =
160+
ok(),
149161
saveIncomingRepair: proc(
150162
channelId: SdsChannelID, msgId: SdsMessageID, entry: IncomingRepairEntry
151-
) {.async: (raises: []).} =
152-
discard,
163+
): Future[Result[void, string]] {.async: (raises: []).} =
164+
ok(),
153165
removeIncomingRepair: proc(
154166
channelId: SdsChannelID, msgId: SdsMessageID
155-
) {.async: (raises: []).} =
156-
discard,
157-
dropChannel: proc(channelId: SdsChannelID) {.async: (raises: []).} =
158-
discard,
167+
): Future[Result[void, string]] {.async: (raises: []).} =
168+
ok(),
169+
dropChannel: proc(
170+
channelId: SdsChannelID
171+
): Future[Result[void, string]] {.async: (raises: []).} =
172+
ok(),
159173
loadAllForChannel: proc(
160174
channelId: SdsChannelID
161-
): Future[ChannelSnapshot] {.async: (raises: []).} =
162-
return ChannelSnapshot(),
175+
): Future[Result[ChannelSnapshot, string]] {.async: (raises: []).} =
176+
ok(ChannelSnapshot()),
163177
)

sds/types/reliability_error.nim

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ type ReliabilityError* {.pure.} = enum
55
reSerializationError
66
reDeserializationError
77
reMessageTooLarge
8+
rePersistenceError ## A persistence backend operation (read or write) failed.

0 commit comments

Comments
 (0)