NEEDS_REBASE_FOR_V0.10: allow for custom servers

[buschtoens]

Allows users to optionally define their own custom server in their configuration.

// config/local.js
module.exports = {
  server: require("spdy").createServer(require("./spdy-options.json"))
};

Closes #737. Relates to #80.

[buschtoens]

@mikermcneil Any thoughts on this? :)

[buschtoens]

I don't want to be one of those guys, who bumps their PRs, but I'd really like to see this upstream anytime soon.
@particlebanana?

[sgress454]

Sorry for the delay. Can you create a new branch in your fork and check in an example app that uses a custom server? Just switching over to Spdy like in your example seems to break all of the Sails routing. Thanks!

[buschtoens]

Thanks for the reply! :)

Of course. I'll do one tomorrow after school (~16:00 UTC+2). Gotta catch some sleep. Haha.

[mikermcneil]

@silvinci @sgress454 this breaks things because, in order for this to work, the express middleware being applied in lib/express would need to be moved upstream, and we'd need to remove the express dependency for HTTP entirely.

I think a more practical solution for the short term might be replacing the https or http server that createServer is being called on with the spdy server, and maintaining the underlying express dependency.

[mikermcneil]

@silvinci going to go ahead and close this one just to keep myself sane, but I'd love to see this rebased where it will still allow cookies and sessions and things to work. Thanks!

[mikermcneil]

See #737 (comment) for update on what's been discussed in the mean time

[mikermcneil]

@silvinci so to summarize the content of #737 and discussion above, what makes the most sense for now is to add SPDY support to our http hook by default. If you wouldn't mind setting that up, I'm on board!

[buschtoens]

@mikermcneil Sorry man, was super troubled in the last week. Preparations for school tests and starting a business at the same time is heavy duty. Haha.

However, if I understand correctly, you would not want users to push in a custom server (instance of http.Server), but rather have them choose a server boilerplate through config?

[mikermcneil]

@silvinci no problem!

I think it's fine for us to add the ability to pass in a custom server- I'm just saying we might as well go ahead and add SPDY support by default, since the module falls back to HTTP anyways. Unless I'm missing something :)

[buschtoens]

Sails breaks the common Node.js convention of passing around HTTP servers or middleware that can be used in such. When doing advanced network stuff it is the most convenient way to just use the middleware and set it up with whatever server(s) you need.

I would suggest the following. As the network (and therefore server) settings are machine-dependent, they belong into config/local.js - just as port already is. I'd introduce two new fields called serverType, which defaults to "http", and serverConfig, which defaults to {}.

• "http": sails will set up an ordinary HTTP server as currently done now.
• "https": sails will set up an ordinary HTTPS server. Setting serverConfig is required for obvious reasons.
• "spdy": sails will set up an SPDY server. Setting serverConfig is required for obvious reasons.
• require("some-server"): sails will use the provided class, which must inherit from http.Server.
• function(app, [port], [config]) { ... }: sails will pass the express app (which is nothing other, but a middleware) to the function and optionally, if set, the port field.
  However, when using this custom function, users most likely won't use port and serverConfig.

The last two options will only be used by advanced users.
AFAIK, node-spdy falls back to regular HTTPS. So you'll want a HTTP server running alongside with the SPDY/HTTPS server. To achieve this, serverType can be an array - respectively the same goes for serverConfig and port.

[mikermcneil]

@silvinci sounds good to me! Only thing I'd add is that we currently have an ssl config option (see https://github.com/balderdashy/sails/blob/master/lib/hooks/http/initialize.js#L37) which I'd like to maintain backwards compatibility for.

[mikermcneil]

Closing this to tidy up- but if there is still interest in this, I'm supportive, lets do it