Skip to content

Commit 53a1a7a

Browse files
committed
fix(sdk,net): apply Gemini review feedback (parallel to sdk-rust#42)
- toHostTimeout helper validates + converts. Rejects 0, NaN, negative, non-finite — matches std::net::TcpStream::set_{read,write}_timeout semantics where Duration::ZERO is an error. - setReadTimeout / setWriteTimeout both go through the helper. - readBytes / peek docstrings made the EOF-vs-would-block contract explicit (empty Uint8Array = EOF; would-block surfaces as a SysError caller can pattern-match). IPv6 bracket fix from the sdk-rust review doesn't apply here — JS connect(host, port) takes them as separate args.
1 parent a501691 commit 53a1a7a

1 file changed

Lines changed: 46 additions & 8 deletions

File tree

packages/astrid-sdk/src/net.ts

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,26 @@ export class SendError extends Error {
5959
}
6060
}
6161

62+
// ---------------------------------------------------------------------------
63+
// Internal helpers
64+
// ---------------------------------------------------------------------------
65+
66+
/**
67+
* Validate + convert a timeout to the host's `bigint | undefined`.
68+
* Mirrors Rust SDK's `to_host_timeout`. Rejects `0` (would be
69+
* ambiguous with "no timeout") matching
70+
* `std::net::TcpStream::set_read_timeout`'s `Duration::ZERO` rule.
71+
*/
72+
function toHostTimeout(timeoutMs: number | undefined): bigint | undefined {
73+
if (timeoutMs === undefined) return undefined;
74+
if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) {
75+
throw SysError.api(
76+
`timeout must be a positive integer (got ${timeoutMs}); use undefined to clear`,
77+
);
78+
}
79+
return BigInt(Math.floor(timeoutMs));
80+
}
81+
6282
// ---------------------------------------------------------------------------
6383
// Handles
6484
// ---------------------------------------------------------------------------
@@ -132,8 +152,15 @@ export class StreamHandle {
132152

133153
/**
134154
* Read up to `maxBytes` without length-prefix framing. Mirrors
135-
* `std::net::TcpStream::read`. Returns an empty Uint8Array on EOF.
136-
* Honours any timeout set via {@link setReadTimeout}.
155+
* `std::net::TcpStream::read`.
156+
*
157+
* Contract:
158+
* - **Empty Uint8Array = EOF** (peer disconnected). Unambiguous.
159+
* - Non-empty = data read (may be shorter than `maxBytes`).
160+
* - Throws `SysError` with message containing `"would block"` if a
161+
* read timeout was set via {@link setReadTimeout} and expired
162+
* with no data. With no timeout set, blocks until data, EOF, or
163+
* capsule unload.
137164
*/
138165
readBytes(maxBytes: number): Uint8Array {
139166
this.#requireOpen();
@@ -145,7 +172,8 @@ export class StreamHandle {
145172
/**
146173
* Write `data` without framing. Returns bytes written (may be less than
147174
* `data.length` when the kernel's socket buffer is full). Honours any
148-
* timeout set via {@link setWriteTimeout}.
175+
* timeout set via {@link setWriteTimeout}; with no timeout set, blocks
176+
* until the write completes or the peer disconnects.
149177
*/
150178
writeBytes(data: Uint8Array): number {
151179
this.#requireOpen();
@@ -154,7 +182,8 @@ export class StreamHandle {
154182

155183
/**
156184
* Peek up to `maxBytes` without consuming them — the next
157-
* {@link readBytes} returns the same data again.
185+
* {@link readBytes} returns the same data again. Same EOF /
186+
* would-block semantics as {@link readBytes}.
158187
*/
159188
peek(maxBytes: number): Uint8Array {
160189
this.#requireOpen();
@@ -191,10 +220,15 @@ export class StreamHandle {
191220
return callHost(`net.nodelay(${this.id})`, () => hostNodelay(this.id));
192221
}
193222

194-
/** Set the read timeout (milliseconds). `undefined` clears it. */
223+
/**
224+
* Set the read timeout (milliseconds). `undefined` clears the
225+
* timeout (reads block indefinitely). `0` is rejected — matches
226+
* `std::net::TcpStream::set_read_timeout` which errors on
227+
* `Duration::ZERO`.
228+
*/
195229
setReadTimeout(timeoutMs: number | undefined): void {
196230
this.#requireOpen();
197-
const ms = timeoutMs === undefined ? undefined : BigInt(timeoutMs);
231+
const ms = toHostTimeout(timeoutMs);
198232
callHost(`net.setReadTimeout(${this.id}, ${timeoutMs})`, () =>
199233
hostSetReadTimeout(this.id, ms),
200234
);
@@ -207,10 +241,14 @@ export class StreamHandle {
207241
return v === undefined ? undefined : Number(v);
208242
}
209243

210-
/** Set the write timeout (milliseconds). `undefined` clears it. */
244+
/**
245+
* Set the write timeout (milliseconds). `undefined` clears it;
246+
* `0` is rejected (matches
247+
* `std::net::TcpStream::set_write_timeout`).
248+
*/
211249
setWriteTimeout(timeoutMs: number | undefined): void {
212250
this.#requireOpen();
213-
const ms = timeoutMs === undefined ? undefined : BigInt(timeoutMs);
251+
const ms = toHostTimeout(timeoutMs);
214252
callHost(`net.setWriteTimeout(${this.id}, ${timeoutMs})`, () =>
215253
hostSetWriteTimeout(this.id, ms),
216254
);

0 commit comments

Comments
 (0)