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 simple templating to error pages? #192

Open
msabramo opened this issue Jan 1, 2015 · 9 comments
Open

Add simple templating to error pages? #192

msabramo opened this issue Jan 1, 2015 · 9 comments

Comments

@msabramo
Copy link
Contributor

msabramo commented Jan 1, 2015

The current error pages are completely static.

Although you can customize them by specifying server.staticDir, they are still static files.

For tsuru which is a PaaS that uses hipache to route to applications, I'd like to be able to see the application name when hipache serves an error page. See:

tsuru/tsuru#970

My idea was to make the error pages use a very simple templating engine (any preferences for which one?) so that the error pages could have very simple dynamic content like the URL or a part of a URL (e.g.: for http://application.example.com, report "application").

I'm thinking performance overhead of the templating is of no concen since this is for error pages which should be accessed infrequently.

Is this a reasonable idea? What would you use for templating?

@willdurand
Copy link
Contributor

👎 static files are fast and safe, adding templaring vars would not fit these two advantages..

@msabramo
Copy link
Contributor Author

msabramo commented Jan 4, 2015

Well yeah static files are fastest and safest.

However the case I'm interested in here is error pages - namely:

  1. Error page when client hits a vhost that hipache doesn't yet know about.
  2. Error page when client hits a vhost that has no available backends.

This is in the context of a PaaS (tsuru) when a developer registers an application and has not deployed it to any nodes for example. They are not going to be hitting this page a lot. But when they do, it would be nice if the error page could tell them what vhost it was that hipache was dealing with. It may not be obvious, because the request could be going through another router or load balancer that maps requests to a vhost that the user never sees from the outside.

So this case could benefit from having the ability to display a small amount of dynamic info and it wouldn't be particularly concerned with squeezing out every last drop of performance.

What I had in mind is very minimal. Not a full-blown templating language like Jinja2. Something logic-less and fast like Mustache or perhaps even as simple as just doing string.replace on a few predefined variables. Should be very fast and secure.

@msabramo
Copy link
Contributor Author

msabramo commented Jan 4, 2015

See #193 to see how minimal I mean. Rough cut since I did it on an iPhone. 😄

@msabramo
Copy link
Contributor Author

msabramo commented Jan 4, 2015

I'd be surprised if doing string.replace a handful of times takes time comparable to doing a query to Redis.

@msabramo
Copy link
Contributor Author

msabramo commented Jan 4, 2015

See tsuru/tsuru#970 for details about my motivation for this.

@willdurand
Copy link
Contributor

While I may get your point, I dont see any benefit since the vhost name is equal to the host users want to browse. So getting a "application not responding" error page seems already pretty clear.

@msabramo
Copy link
Contributor Author

msabramo commented Jan 4, 2015

Thanks for your comments!

I agree that what you said would be true if accessing hipache directly.

But in my case, there is a layer of indirection. The user goes to a URL like http://testenv/ or http://testenv/getstarted, which is an nginx server that proxies various paths to different vhosts - e.g.:

/ => http://anonweb.paas.dev/
/getstarted => http://profileweb.paas.dev/getstarted
/upgrade => http://billweb.paas.dev/upgrade

The *.paas.dev vhosts are hipache.

So with this setup, if a user goes to http://testenv/getstarted and gets a hipache error, they don't know that it's because of a problem with the "profileweb" vhost.

This is why I want to add the vhost to the error page.

I hope that makes sense?

@willdurand
Copy link
Contributor

It does. However it still seems to be a very specific use case, and I'd rather go for a node-http-proxy middleware for that (if static pages are handled by this proxy in the code, don't have the code in mind).

@msabramo
Copy link
Contributor Author

msabramo commented Jan 4, 2015

Yeah, agreed that this is a very specific use case. And I'm starting to think maybe doing string substitution is a bit too much to solve this small problem, unless there were other uses for the string substitution.

I'm now thinking about just adding a small bit of code to add the vhost to the output. That would be simpler and doesn't require inventing a substitution syntax or modifying the templates.

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