add handling for unset request types in endpoint

[OsakiTsukiko]

No description provided.

[renerocksai]

Please elaborate. There already exists nop() for unset handlers. So this patch adds a kind of "and set all the other ones to this function here" - which I do not like. I like it to be explicit. It's 7 fields to set in total.

[renerocksai]

So, at the moment, I see it as a matter of taste. But if you can give me a compelling argument, I might reconsider "my taste" :-)

[OsakiTsukiko]

oh, it's just for ease of use, i needed a way to set all unused handlers to a 404 handler and setting all the other fields to my handler didn't occur to me for some reason only after making this 😭 but yea, its just so you can easily set a function to catch all unset handlers so you don't forget one or something. I guess it would be more useful if there were no nop and it would force you to handle all requests so that you don't have hidden behavior, for me at first I was expecting the main listener to catch this instead of the endpoint and I did not understand why it was a valid request but I didn't get any output, if this makes any sense.

[renerocksai]

Yes, it makes total sense! You make a convincing consistency-argument here!

My suggestion:

• we leave the nop() in there. But we make it a pub fn, so you can provide .unset = zap.Endpoint.nop at Endpoint.init.
• the orelse chains could be modified like this:

  .get = s.get          // line-breaks just for GitHub comment
         orelse s.unset 
         orelse @compileError("Endpoint handler `.get` is unset, and no `.unset` handler is provided.")
  

WDYT? Maybe we should even rename nop() to nop_handler(), to make its name more descriptive.

[OsakiTsukiko]

Sounds good. I'll get to changing it right away.

[OsakiTsukiko]

added the requested changes, went for @panic instead of @compileError, the former not forcing comptime and changed nop to dummy_handler.

[renerocksai]

Thank you, your implementation is great! And sorry it took so long for replying. I had some changes in mind for a while now and eventually am getting to implementing them. One consequence is that I am getting rid of the endpoint settings containing handler function pointers. Instead, I treat "Endpoints" more like a contract / interface: you need to define the get/post/delete/... functions orelse it won't fulfill the contract (checked at comptime). That is more explicit and less magic and less indirect - less room for confusion and hence less coding-around-confusabilities.

[OsakiTsukiko]

🥲 Oh, should i close the pr then? I'm guessing this is no longer needed?

[renerocksai]

Yeah, I am moving away from providing callbacks via settings, to using them straight from the Endpoint struct.

Maybe there is a case for providing way to easily set handlers to nop() so they get some sane default implementation. But that's for a different PR.

I value your contribution and it has certainly brought more of my awareness to this stuff - and I eventually decided to rework the whole Endpoint concept 😄

[OsakiTsukiko]

Ok. Looking forward to see the new system? Any idea when it's gonna be finished?

[renerocksai]

Oh, it's currently on master. Just merged the PR; just need to write more about it, to explain it / the changes, and release it.

The new zap.Endpoint stuff works like this, via example of error.zig that creates the ErrorEndpoint:

//!
//! An ErrorEndpoint
//!
const std = @import("std");
const zap = @import("zap");

/// A simple endpoint listening on the /error route that causes an error on GET
/// requests, which gets logged to the response (=browser) by default
pub const ErrorEndpoint = @This();

path: []const u8 = "/error",
error_strategy: zap.Endpoint.ErrorStrategy = .log_to_response,

pub fn get(_: *ErrorEndpoint, _: zap.Request) !void {
    return error.@"Oh-no!";
}

// unused:
pub fn post(_: *ErrorEndpoint, _: zap.Request) !void {}
pub fn put(_: *ErrorEndpoint, _: zap.Request) !void {}
pub fn delete(_: *ErrorEndpoint, _: zap.Request) !void {}
pub fn patch(_: *ErrorEndpoint, _: zap.Request) !void {}
pub fn options(_: *ErrorEndpoint, _: zap.Request) !void {}

• no @fieldParentPtr: Endpoints now directly get their @This() pointer passed into their methods
• request handlers are allowed to return errors!
• the error_strategy decides if errors are logged to console or reported as HTML to the client (debugging in browser)
• no "Settings":
  • path and error_strategy are required for Endpoints
  • all http method handlers must be present, but of course may be empty
  • all of the above are checked at comptime, with meaningful compile error messages