fix: various connection bugs #25
Conversation
| transport.send(m); | ||
| } | ||
| })(); | ||
|
|
||
| // transport -> output | ||
| const listener = (msg: OpaqueTransportMessage) => { | ||
| if (belongsToSameStream(msg)) { | ||
| if (isStreamClose(msg.controlFlags)) { |
There was a problem hiding this comment.
The "stream close" msg wouldn't also contain normal message contents, correct?
There was a problem hiding this comment.
yup, stream close on the client means that message is the control message (it's kind of weird because its not the case for the server, can probably add an extra check here to make it consistent)
| await stream.promises.inputHandler; | ||
| stream.outgoing.end(); | ||
| await stream.promises.outputHandler; |
There was a problem hiding this comment.
can we await these together?
Promises.all()
There was a problem hiding this comment.
don't think it can actually, the input handler requires input to be ended but output to be open whereas output requires output to be ended
| // we ended, send a close bit back to the client | ||
| // only subscriptions and streams have streams the | ||
| // handler can close |
There was a problem hiding this comment.
client stream can't be closed by server? what if it's like "ah you're sending me too much data!" I guess then it would instead be "just use a stream"
There was a problem hiding this comment.
client stream hasn't been implemented yet haha
| } catch (err) { | ||
| errorHandler(err); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
i think we should throw here on unhandled procedure types
There was a problem hiding this comment.
would probably allow us to not have to set inputHandler to a resolved promise initially, which feels a bit weird.
| this.ws.onmessage = (msg) => this.onMessage(msg.data as Uint8Array); | ||
|
|
||
| // take over the onmessage for this websocket | ||
| this.ws.onmessage = (msg) => transport.onMessage(msg.data as Uint8Array); |
There was a problem hiding this comment.
would be nice to remove these as eventually
ws.onmessageneeded more documentation on what it was doingTransportandConnection(with a diagram!)WebSocketServerTransportshouldn't close the underlyingwsson.close()(but it should terminate all connected clients)