Sync tool library #4290
Sync tool library #4290
Conversation
packages/nexus/src/channel.ts
Outdated
| this.bufferedAcks.push(id) | ||
| resolve(true) |
There was a problem hiding this comment.
I think there's a case to be made that this should resolve only once the buffered ack is processed. I don't think it would be a very big change if we prefer that.
| handler: NexusHandler, | ||
| wsOpts: NexusWebsocketOptions = {}, | ||
| ) { | ||
| this.abortController = new AbortController() |
There was a problem hiding this comment.
Nit: You could init this at the declaration to make things less verbose:
private readonly abortController = new AbortController()| await response.arrayBuffer() | ||
|
|
||
| if (!response.ok) { | ||
| await response.arrayBuffer() |
There was a problem hiding this comment.
Body was already consumed above
| await response.arrayBuffer() |
| }, | ||
| body: JSON.stringify({ dids }), | ||
| }) | ||
| await response.arrayBuffer() |
There was a problem hiding this comment.
Nit: A more resource efficient way to drain would avoid putting it memory (though in practice it doesn't probably matter)
await response.body?.cancel()https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/cancel
Cancel is used when you've completely finished with the stream and don't need any more data from it, even if there are chunks enqueued waiting to be read.
| await this.handler.onEvent(evt, { signal: this.abortController.signal }) | ||
| await this.ackEvent(evt.id) |
There was a problem hiding this comment.
Would it make sense for the event handler to actually perform the ack ?
| await this.handler.onEvent(evt, { signal: this.abortController.signal }) | |
| await this.ackEvent(evt.id) | |
| await this.handler.onEvent(evt, { | |
| signal: this.abortController.signal, | |
| ack: async () => this.ackEvent(evt.id) | |
| }) |
Client lib for bluesky-social/indigo#1170
This is mostly meant to get a sense for how the tool feels to use