On Fri, 26 Jul 2013 12:12:11 GMT, Sönke Ludwig wrote:

On Fri, 26 Jul 2013 11:19:39 GMT, Manu Evans wrote:

So, here are some first impressions/issues that came up immediately.
Perhaps I'm doing it wrong, and if so, I'd appreciate guidance.

1) req.form, req.query, etc, maps throw if you try to access an element that isn't there. This leads to endless spam like this: 

 if("xys" in req.form)
 {
   string xyz = req.form["xyz"];
   ...
 }

or 

 string xyz = "xyz" in req.form ? req.form["xyz"] : null;

That's ugly and annoying. Is that really the accepted pattern?

Surely: 

 string xyz = req.form["xyz"];

...is better, and xyz should just be assigned null if not present?

You also have req.form.get("xyz", null) which does exactly that (see http://dlang.org/hash-map.html). Since this is standard D AA behavior, I didn't want to deviate from that potentially creating confusion.

Ah right, I knew I must be missing something obvious! (I've never used a builtin AA before, I didn't spot the get() method.

2) HTTPServerRequest has no way to carry user data. URL router is a linear process, and functions like 'check login' that need to look up a user profile should be able to carry that profile across into the following handler. Otherwise the actual handler just has to look up the same user again.
Ie, there is no way to carry state through the URL route. I have a few situations of this sort where it would be simpler and more efficient to carry some state.

The HTTPServerRequest.params field is meant for such cases where data needs to be passed down the request handler chain. Unfortunately for historic reasons it is a string[string] map, although it should rather be Variant[string]. The problem here is just finding a transition that doesn't break all existing code (adding an additional field would be the easy option, but is also kind of ugly unless it actually has a better name...).

I see... Well I'll leave it with you.

3) Fiber Local Storage. Since TLS isn't reliable in the vibe.d environment, an explicit solution should be offered, otherwise users are likely to try using TLS and run into unexpected problems...

There is vibe.core.core.setTaskLocal and getTaskLocal for task local storage. They use a Variant[string] map internally, which is not ideal regarding performance (incl. memory allocations). But a really good solution would probably require some form of compiler support.... well thinking about it, maybe not. It should be possible to create a TaskLocal!T template which registers a slot in a linear per-task memory area.

Yeah, I think a good library implementation is possible. I'll use your API for now.

Task local storage is of course the obvious alternative for 2) until the params field can carry arbitrary types.

Indeed, although they are subtly different in feel/usage. I'll see how I go with that.

BTW 2) and 3) have come up now a number of times, so I'll see how their visibility can be improved in the documentation.

Cool.

1.2) It's pretty lame that all the examples on the website do stuff like: 

res.redirect("/users/"~req.post["user"]);

With no comment to note that the application will CRASH if post["user"] is not present. My presumption was that it would just be handled as a null string.

It should actually be caught by the HTTP server and result in a 500 error. However, on Windows and especially in the Visual Studio debugger, any kind of exception can sometimes be unrecoverable due to some issues with the injected exception handler or other issues that I didn't really track down. Maybe that is what you've seen? Anyway, proper input verification would surely be in order for the examples.

Yeah, well this is also just a reflection of my n00b-ness. Although I reckon anyone new to D will make the same mistake!

And now a new issue that has just come up...

My application that makes HTTP requests seems to lock up vibe.d. It seems to sit there waiting for more data over the socket, even though it's received all that it's going to.
Then my app disconnects the socket and vibe.d dies inside: 

auto reqln = cast(string)stream.readLine(MaxHTTPHeaderLineLength, "\r\n", alloc);

The thing that caught my eye immediately was the "\r\n" thing. That's not the HTTP standard is it?
My app sends the HTTP request with "\n" separating the lines. Other HTTP servers accept my requests just fine, but vibe.d locks up... weird.

I tried changing my app to use "\r\n" line separators instead of "\n", and vibe.d started working as expected...

what's the deal?