Request: Provide a writer interface to the web socket

[zigster64]

... mainly so we can use fmt.print() style writes to the web socket connection, without having to jump through hoops.

This would be handy (for me), because Im generating content over the web socket that is output from my templating tools, which use a writer interface.

I did a simple and half-baked implementation in my fork (at least it compiles and generates output), that attempts to upgrade server.Conn to a writer .... but getting the framing right is slightly non-trivial, and subjective. Just mapping write() directly to writeFrame() for example is a bad move. Would really want to buffer the writes up and provide flush operation to create a single output frame. hmmm.

It would also mean the function signature for the server.Conn.write() function changes to return a usize instead of void, which would be a breaking change.

Maybe a better idea would be to provide a higher level wrapper over the connection that provides a writer instead ?

In the meantime, where Im wanting to do this, Im just creating a buffer, writing to that, and then outputting the buffer to the server.conn when Im done. That works for me, and is not that big a deal.

Would be nice to have the web socket library do all that for you, as it would behave more like http.zig as well.

[karlseguin]

I'm happy to merge this if you think it looks ok.

https://github.com/karlseguin/websocket.zig/tree/conn_writer#writer

The flush is the issue. I don't think there's a way around creating 2 layers - one to get our Conn-specific writer (which has a flush) and another to get an std.io.Writer.

The one thing I'm happy about this implementation is that it can leverage the existing BufferPool. It might still need to dynamically allocate, but having it at least try to use the existing buffers is a win in my book.

[zigster64]

Excellent ... that looks good to me. Yeah sorry, it sounded simple enough, but it's a bit more work than it first appears.

Only possible edge case I can see (I think) is that if the buffer grows due to large writes AND something aborts the current function before the wb.flush() call, it may lead to a leak ?

The example in the docs is OK

var wb = try conn.writeBuffer();
try std.fmt.format(wb.writer(), "it's over {d}!!!", .{9000});  // <<<-- if this fails, then nothing got allocated, so there is no leak
try wb.flush(.{});

but if the user adds more writes like this :

var wb = try conn.writeBuffer();
// not doing a wb.deinit() here, like the previous example
try std.fmt.format(wb.writer(), "something big enough to trigger an allocation ....", .{});
try std.fmt.format(wb.writer(), "it's over {d}!!!", .{9000});  // <<<-- if this fails, then the previous alloc leaks
try wb.flush(.{});

Yeah fair enough, that's a normal Zig gotcha, if the user is lazy and forgets to deinit() the writeBuffer

Merge it I reckon, looks clean to me

[karlseguin]

deinit on flush was meant as a convenience. Your edge case is valid, and now i'm not sure. Removing this behavior would mean you always do:

var wb = try conn.writeBuffer();
defer wb.deinit();
...
try wb.flush(.text);

[karlseguin]

Merged to master. Removed the auto-deinit. Needs to be explicit now (which plays well with defer).

[zigster64]

server.zig line 231

pub fn writeFramed()

self.steam << typo ! should be self.stream ?

Compiler + tests don't pick it up because it's not employed anywhere yet.

[karlseguin]

Fixed and added a test for it.

[zigster64]

Updated my app to use this, replacing my other buffer. Seems to work great :)

The flush api still feels a bit odd ? Maybe it makes more sense to declare the frame type up front in the constructor, rather than the flush.

// pass the frame type up front
var wb = try conn.writeBuffer(.text);
defer wb.deinit();
try std.fmt.format(wb.writer(), "it's over {d}!!!", .{9000});  
try wb.flush();

Would be nice to just auto-flush on deinit as well. This would need deinit() to return !void, which would then interfere with defer. I haven't seen a clean way around this yet.

Nice work

[karlseguin]

I agree with the change to the op_code. it's on writeBuffer now.

I think flush is the best of the options. In addition to the try in defer... I think having deinit write to the socket, which is the main outcome you're after, a bit unexpected.