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

New version of server.js doesn't work with iisnode #9

Closed
sirws opened this issue May 1, 2015 · 10 comments
Closed

New version of server.js doesn't work with iisnode #9

sirws opened this issue May 1, 2015 · 10 comments

Comments

@sirws
Copy link
Contributor

sirws commented May 1, 2015

I just installed the latest version of koop-sample-app and put it in my iis server with iisnode. I get an internal server error.

If I change this line:
http.createServer(app).listen(config.server.port);
to
http.createServer(app).listen(process.env.PORT || config.server.port);

It works. Is there a reason this was taken out server.js? It was there before. Thanks!

@sirws
Copy link
Contributor Author

sirws commented May 5, 2015

@chelm Is there any issue updating this in the master so I don't have to modify it every time there is an update?

@jgravois
Copy link

jgravois commented May 6, 2015

that should be fine. can you PR it? if not, I can.

@chelm
Copy link
Contributor

chelm commented May 6, 2015

👍

sirws added a commit to sirws/koop-sample-app that referenced this issue May 6, 2015
Change to support koop and iisnode.
@sirws
Copy link
Contributor Author

sirws commented May 6, 2015

#10

ungoldman added a commit that referenced this issue May 6, 2015
@ungoldman
Copy link
Contributor

config.server.port shouldn't be standard anyway as that's something that should be determined at the app (express) level not the middleware (koop) level.

@chelm
Copy link
Contributor

chelm commented May 6, 2015

@ngoldman yes and its only used at the app level. That can and should be removed in favor of env.PORT - but also do we have apps default to a port? I guess that'd be no different than having the config default to a port, so... word.

@ungoldman
Copy link
Contributor

I believe this is resolved, closing. Thanks for the PR @sirws 👍

@mhogeweg
Copy link

new to koop/node.js. not seeing http.createServer(app).listen(config.server.port); in the server.js file in the sample app I just cloned. should the sample app install documentation be updated?

@sirws
Copy link
Contributor Author

sirws commented May 20, 2015

@mhogeweg I think their fix means you can skip that step. I would give it a shot without making that fix. I will try it later myself with the new server.js and make sure it doesn't break anything. I don't think it will based on looking at it.

@jgravois
Copy link

we now set the port here

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

5 participants