Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add std.io.Writer and std.io.Reader support for zap.SimpleRequest #66

Open
hauleth opened this issue Jan 9, 2024 · 4 comments
Open

Comments

@hauleth
Copy link

hauleth commented Jan 9, 2024

Currently the only (exposed) way to send response is to create buffer where you write whole response and you send that. However it would be way more useful to have support for std.io.Writer (and potentially std.io.Reader for reading body of the request) as it would allow developer to use std.fmt functions. This however requires changes upstream as calling http_send_body repeatedly sets incorrect content-length header (it will be set to length of last chunk instead of length of the whole written data).

@renerocksai
Copy link
Member

That would be a nice addition if facilio supported it. Can you verify everything works well when you set the content-length after the final write?

My hunch is that after sending the connection is in an invalid state internally.

Another thing to consider is who owns the memory with writer support.

You can always use an arraylist's writer and write into that; then call send on the arraylist's items. That's what I would do and is only 2 lines of code or so. That's why I am not yet convinced of the added value.

Plus, wrapping facil.io, we'd need to somehow keep a reference to the underlying Reader / Writer context and in the case of Writer: buffer / ArrayList. The easiest way would be to wrap this functionality in a separate struct that you pass the Request:

const reader = zap.Reader(request);
var writer = zap.Writer(request);

// use writer

// send the buffer:
writer.flush(); // ?
// or
writer.send(); // ?

// or
r.sendBody(writer.buffer);

Since you're already playing with it, consider submitting a PR ;-)

@hauleth
Copy link
Author

hauleth commented Jan 9, 2024

I have written my wrapper around zap.SimpleRequest where I have:

pub const Writer = std.io.Writer(*const Self, zap.HttpError, sendBodyChunk);

pub fn writer(self: *const Self) Writer {
    return .{ .context = self };
}

pub fn sendBodyChunk(self: *const Self, chunk: []const u8) zap.HttpError!usize {
    const ret = zap.http_send_body(self.req.h, @as(
        *anyopaque,
        @ptrFromInt(@intFromPtr(chunk.ptr)),
    ), chunk.len);
    if (ret == -1) return error.HttpSendBody;
    return chunk.len;
}

And it seems to work quite ok. My bad, that doesn't really work.

@renerocksai
Copy link
Member

Yeah, I figured. But nothing stops you from wriing into an ArrayList, then passing its items to zap.

@renerocksai
Copy link
Member

That would be a nice addition if facilio supported it. Can you verify everything works well when you set the content-length after the final write?

My hunch is that after sending the connection is in an invalid state internally.

Another thing to consider is who owns the memory with writer support.

You can always use an arraylist's writer and write into that; then call send on the arraylist's items. That's what I would do and is only 2 lines of code or so. That's why I am not yet convinced of the added value.

Plus, wrapping facil.io, we'd need to somehow keep a reference to the underlying Reader / Writer context and in the case of Writer: buffer / ArrayList. The easiest way would be to wrap this functionality in a separate struct that you pass the Request:

const reader = zap.Reader(request); var writer = zap.Writer(request);

// use writer

// send the buffer: writer.flush(); // ? // or writer.send(); // ?

// or r.sendBody(writer.buffer);

Since you're already playing with it, consider submitting a PR ;-)

Since facil.io doesn't support chunked responses, the above is what I'd implement. For later reference when I have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants