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

Ports are not always numbers #4413

Closed
christopheranderson opened this Issue May 19, 2015 · 27 comments

Comments

Projects
None yet
9 participants
@christopheranderson

christopheranderson commented May 19, 2015

In Webapp.js, parseInt is used on the env.PORT. The env.PORT need not always be a number, but can be a named pipe.

var localPort = parseInt(process.env.PORT) || 0;

This issue can be hit when running Meteor with IIS on Windows, which uses a named pipe to feed the connections to Node. Removing the parseInt function call works fine for named pipes and ports on local tests. express-generator ships with a port parsing function which is more robust - maybe a better option than dropping the parseInt entirely.

@stubailo

This comment has been minimized.

Contributor

stubailo commented May 19, 2015

Seems pretty legit to me. It's technically a feature request because we don't support production Meteor on Windows, but I would look at a PR for this.

@christopheranderson

This comment has been minimized.

christopheranderson commented May 20, 2015

Good to hear. I'll bake up a PR this week. Thanks.

@Rebolon

This comment has been minimized.

Contributor

Rebolon commented Jul 20, 2015

Port environment var is not used by meteor on windows, is it normal ?

@stubailo

This comment has been minimized.

Contributor

stubailo commented Jul 20, 2015

That shouldn't be the case. Do you have a reproduction?

@Rebolon

This comment has been minimized.

Contributor

Rebolon commented Jul 21, 2015

I've just tryed to reproduce it :

  • cd D:/dev/javascript/projects
  • meteor create issuePortWindows
  • cd issuePortWindows
  • set PORT=3333
  • meteor

then i got this in the console :

[[[[[ ~\d\dev\javascript\projects\issuePortWindows ]]]]]

=> Started proxy.
=> Started MongoDB.
=> Started your app.

=> App running at: http://localhost:3000/
   Type Control-C twice to stop.

i open chrome to localhost:3333 : it fails
i open chrome to localhost:3000 : it succeed

i kill meteor and then relaunch meteor with port params

  • meteor --port 3333

then i got this in the console :
[[[[[ ~\d\dev\javascript\projects\issuePortWindows ]]]]]

=> Started proxy.
=> Started MongoDB.
=> Started your app.

=> App running at: http://localhost:3333/
   Type Control-C twice to stop

i open chrome to localhost:3000: it fails
i open chrome to localhost:3333 : it succeed

I'm using windows 8.1, a meteor 1.1.0.2, and consoleZ 1.11.0.14175

@svda

This comment has been minimized.

Contributor

svda commented Aug 11, 2015

We ran into the same issue when trying to build Meteor as a web app on Azure. In our case the PORT is not an integer, but a named pipe (string). web-app.js converts our string to an integer using parseInt, resulting in an app that doesn't listen on any port. When we remove the parseInt function call, the app is working.

@christopheranderson

This comment has been minimized.

christopheranderson commented Sep 3, 2015

My approval to sign the CLA has been delayed (couldn't you tell? 😢). It should be a relatively simple fix to remove the parseInt on the port. There probably needs to be a test added which uses a named pipe. There is also a risk of errors for existing users who provide a port as a string like "3000foo"; what worked for them before (because of the nature of parseInt), will no longer work. This is likely a corner case and most people aren't doing anything of this nature, but it may be worth a warning in a version change doc.

I'm happy to advise best practices for this if someone wants to tackle this while I'm being hung out to dry. 😄.

@ericterpstra

This comment has been minimized.

Contributor

ericterpstra commented Sep 15, 2015

@christopheranderson I'm interested in helping out with this, and have no trouble signing a CLA. Any thoughts on making this as backwards-friendly as possible? Is there a way to detect the use of IISNode from within Node.js?

@christopheranderson

This comment has been minimized.

christopheranderson commented Sep 16, 2015

The strictest change needed to support named pipes for ports would involve checking the string to see if it looks like a named pipe. Following the MSDN docs for Pipe Names formatting, all named pipes should be named //({Server Name || . }/pipe/{Pipe Name}. A regex to start with could be \//?.+/pipe/.+\.

@ericterpstra

This comment has been minimized.

Contributor

ericterpstra commented Sep 17, 2015

Well I forked meteor and cloned the devel branch to try this out, but was greeted with Sorry, this OS is not supported when trying to run meteor from the cloned repo. I get the feeling that Windows is still very much a hostile environment for Meteor, and we're facing an uphill battle. Anyway, I'll set up a VM or bring in my Macbook tomorrow and try again.

@moonrockfamily

This comment has been minimized.

moonrockfamily commented Sep 17, 2015

I forked the repo with the intent of creating a unit test and corresponding fix for supporting named pipes, but have been struggling with the process. https://github.com/meteor/meteor/wiki/Contributing-to-Meteor didn't really help me with the following:

  • How to run the entire meteor core test suite and ensure the project is healthy?
  • How to run/modify existing tests for 'numbered port' support?
  • How to add new unit tests for 'named pipe port' support?
    • What type of unit test should I create?
  • How to build meteor core and it's packages?
  • How to install the built meteor core and it's packages?

I'm likely missing a critical wiki page or blog that will set me straight!

@stubailo

This comment has been minimized.

Contributor

stubailo commented Sep 17, 2015

@ericterpstra unfortunately we don't support running from Git Bash, you need to run the meteor.bat script from CMD or PowerShell.

@moonrockfamily You should add a unit test in webapp.js somehow, and you can run all of the tests with ./meteor test-packages from the git repo. I don't think we have a good article about the mechanics of running tests, etc.

@moonrockfamily

This comment has been minimized.

moonrockfamily commented Sep 17, 2015

Running meteor test-packages produces the following output to console:
[[[[[ Tests ]]]]]

=> Started proxy.
=> Started your app.

=> App running at: http://localhost:3000/
Type Control-C twice to stop.`

Opening http://localhost:3000/ displays:

All tests pass! and 'Passed 0 or 0'

@stubailo

This comment has been minimized.

Contributor

stubailo commented Sep 17, 2015

You need to run the meteor script inside the repository by typing ./meteor. If you just type meteor, it runs the global installed version.

@ericterpstra

This comment has been minimized.

Contributor

ericterpstra commented Oct 9, 2015

Tried again to work with meteor and run the test suite, but getting an error with node-aes-gcm. Seems to be a 32 vs 64 bit thing. I'm on Windows 7 64-bit, but running Node 0.12.7 32-bit because meteor.bat wouldn't use the right fibers.node with Node x64.

Anyone else made any progress on this? This is only one of several problems I've had trying to run the tests.

@christopheranderson Has that CLA been approved? :)

C:\dev\meteor>.\meteor test-packages
[[[[[ Tests ]]]]]

=> Started proxy.
=> Linted your app. No linting errors.
=> Exited with code: 8
W20151009-16:21:23.943(-5)? (STDERR)
W20151009-16:21:23.944(-5)? (STDERR) C:\dev\meteor\dev_bundle\server-lib\node_modules\fibers\future.js:245
W20151009-16:21:23.944(-5)? (STDERR)                                            throw(ex);
W20151009-16:21:23.944(-5)? (STDERR)                                                  ^
W20151009-16:21:23.944(-5)? (STDERR) Error: The operating system cannot run %1.
W20151009-16:21:23.944(-5)? (STDERR)
W20151009-16:21:23.944(-5)? (STDERR) C:\dev\meteor\.meteor\packages\npm-node-aes-gcm\0.1.3_6\npm\node_modules\node-aes-gcm\build\Release\node_aes_gcm.node
W20151009-16:21:23.944(-5)? (STDERR)     at Module.load (module.js:356:32)
W20151009-16:21:23.944(-5)? (STDERR)     at Function.Module._load (module.js:312:12)
W20151009-16:21:23.945(-5)? (STDERR)     at Module.require (module.js:364:17)
W20151009-16:21:23.945(-5)? (STDERR)     at require (module.js:380:17)
W20151009-16:21:23.945(-5)? (STDERR)     at Object.Npm.require (C:\Users\ET1423~1\AppData\Local\Temp\meteor-test-rununbugc\.meteor\local\build\programs\server\boot.js:162:18)
W20151009-16:21:23.945(-5)? (STDERR)     at Package (packages/npm-node-aes-gcm/packages/npm-node-aes-gcm.js:10:1)
W20151009-16:21:23.945(-5)? (STDERR)     at Package (packages/npm-node-aes-gcm/packages/npm-node-aes-gcm.js:25:1)
W20151009-16:21:23.945(-5)? (STDERR)     at C:\Users\ET1423~1\AppData\Local\Temp\meteor-test-rununbugc\.meteor\local\build\programs\server\packages\npm-node-aes-gcm.js:45:4
W20151009-16:21:23.945(-5)? (STDERR)     at C:\Users\ET1423~1\AppData\Local\Temp\meteor-test-rununbugc\.meteor\local\build\programs\server\packages\npm-node-aes-gcm.js:54:3
W20151009-16:21:23.945(-5)? (STDERR)     at C:\Users\ET1423~1\AppData\Local\Temp\meteor-test-rununbugc\.meteor\local\build\programs\server\boot.js:242:10
@ericterpstra

This comment has been minimized.

Contributor

ericterpstra commented Oct 12, 2015

I managed to get everything working and tested. Basically added a check to see if process.env.PORT matches the named pipe pattern and returns a string. If not, it falls back to parseInt.

The tests passed locally, but something failed on CircleCI related to CSS minification. Not sure why.

@stubailo @christopheranderson - If there's anything to change or add, please let me know. Looking forward to getting this fixed.

@christopheranderson

This comment has been minimized.

christopheranderson commented Oct 13, 2015

Looks good.

I think that this addresses the Named Pipe issue while allowing the least amount of potential functional changes otherwise. In a perfect world, we might throw with a "Ports must be a number or match the named pipe pattern", but that would probably cause some headaches while only helping the rare person who gives a bad port. I think how you have it is fine.

If you want really pedantic feedback, :), I'd add a test for a garbage string and expect it to fail.

@stubailo stubailo closed this in 412e502 Oct 13, 2015

@stubailo

This comment has been minimized.

Contributor

stubailo commented Oct 13, 2015

Hopefully the next release is soon!

@mark-alfonso

This comment has been minimized.

mark-alfonso commented Oct 13, 2015

Hi Guys, can someone post a blog or any reference for deploying meteor 1.2 in a Windows Server 2012? It's just a pain in the ass knowing that every sites I visit the tutorials are for Ubuntu and/or Mac. Btw, I have no choice but to use Windows Server 2012 ;(

@ericterpstra

This comment has been minimized.

Contributor

ericterpstra commented Oct 13, 2015

@ICS-user I've had to piece things together. The biggest pain (aside from this port issue) is getting fibers compiled. I'm working on a deployment for Windows Server 2012 right now, and will post something online or in the forums in the next few days. (I'm also 'stuck' with WS2012 at work)

@mark-alfonso

This comment has been minimized.

mark-alfonso commented Oct 13, 2015

@ericterpstra I share the agony. I need to deploy my app before Friday. Please let me know if you are already done with that post. Thanks!

@hippien

This comment has been minimized.

hippien commented Oct 23, 2015

@ericterpstra I'm very interesting in your work of running meteor with iisnode. Where will you post regarding this matter so I can follow your progress?

@christopheranderson

This comment has been minimized.

christopheranderson commented Oct 24, 2015

Wrote this a while back for a customer on trying to run it on IISNode with Azure Web Apps. Let me know if it helps. It requires demeteorize, which isn't ideal.

Making Meteor.js work on Azure Web Apps

Prepare dev environment:
0. Follow instructions for installing demeteorize found on their github: https://github.com/onmodulus/demeteorizer

  1.  Install node 0.12.6 32bit on a Windows machine
    
  2.  Create your meteor app as normal
    

Preparing package for deployment to Azure:
3. Navigate to directory containing meteor app and run “demeteorizer”
a. You can optionally output to a dedicated folder with “demeteorizer –o ./path/to/folder”
4. (TEMPORARY FIX) Remove ParseInt call on Port in line 763 so that the line reads just:
a. var localPort = process.env.PORT

5. Specify your node engine version in your package.json (FYI: https://azure.microsoft.com/en-us/documentation/articles/nodejs-specify-node-version-azure-apps/)

Testing locally:
6. Set up your local variables
a. MONGO_URL='mongodb://user:password@host:port/databasename?autoReconnect=true'
b. ROOT_URL='http://localhost:8080'
c. PORT=8080 (optional, defaults to 80)
7. Start your app with “node main.js”

Setting up and deploying to Azure:
8. Create your Web App (Article for reference: https://azure.microsoft.com/en-us/documentation/articles/web-sites-nodejs-develop-deploy-mac/)
9. Set up your application variables (note: no port)
a. MONGO_URL='mongodb://user:password@host:port/databasename?autoReconnect=true'
b. ROOT_URL='http://mymeteorapp.azurewebsites.net'
10. Use git or ftp deploy from a Windows machine with the node_modules included (don’t use Azure Websites automated package deployment stuff)
11. Now, browse your site!

There's some investigation I'm looking into of using the iisnode.yml file to specify starting the meteor site without the "demeteorize" step, but it's in the "nice to do" pile. @rramachand21 is a great resource if you run into issues with IISNode specifically.

@mark-alfonso

This comment has been minimized.

mark-alfonso commented Oct 25, 2015

Hi @ericterpstra where are you on the writings for your deployment. It's a good thing that my client agreed to deploy the web app in the meteor cloud. But not for long, I will be needing to deploy it on a Windows provided server. So I was hoping I can reference you writing for this matter. All the best!

@mark-alfonso

This comment has been minimized.

mark-alfonso commented Oct 25, 2015

hi @christopheranderson regarding the azure. Is there any way that we can deploy by just using a plain cloud instance of Windows Server 2012? With IISNode? My client's set up is just like that. Thanks!

@christopheranderson

This comment has been minimized.

christopheranderson commented Oct 26, 2015

You gotta drop your site into the right place, have a web.config & associated files to make IISNode work, and set your app settings correctly as above. The requirements aren't really different from Azure Web Apps; it's just a pain to manage your own VMs. Auto-scale/heal ftw.

@nielsweistra

This comment has been minimized.

nielsweistra commented Oct 26, 2015

Recently i described the steps how to install a Meteor App on IIS using IISNODE. https://forums.meteor.com/t/anyone-have-meteor-working-with-window-server-iis-iisnode/8922/2

We also deploying it to Azure WebApps. To build the package we're using Teamcity to automate the steps to make it able to run on Windows based enviroments and indeed i'm using MSDeploy to package and deploy the solution to internal machines or Azure WebApps or VM's.

When using MSdeploy aka WebDeploy you have to include the web.config and the iisnode.yml. If you're deploying using GIT it will check in the root of the meteor app if there is a package.json to check if it's a NodeJS application. If so, Kudu (the mechanism behind Azure WebApp) will create a web.config with some default rewrite rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment