tsuru / platforms

Official platforms for the tsuru PaaS.
BSD 3-Clause "New" or "Revised" License
76 stars 70 forks source link

platform nodejs error parsing node version #54

Open vmalaga opened 7 years ago

vmalaga commented 7 years ago

Hi

I have a developer pushing a nodejs app and is getting error because the engines.node version is malformed

"engines": {
-    "node": ">= 8.6.0"

with the >= the deploy script returns error and exit

remote: error running "/var/lib/tsuru/deploy archive http://172.16.70.100:3232/?id=21eb855af295f8432dfc22f656786aa9415cf3e9d77278380806edf23f3e725d375f00f32e8e497e4fc51a31ade46b3730da8e6cbad1bd1f431802fb08cc1e0c": exit status 3
remote: Exit status 1

also, this is dificult to debug and would be nice to add verbose deploys #39

ggarnier commented 7 years ago

@vmalaga does it make sense to set a range for the node version in an app, instead of setting a specific version? According to npm docs, this field is advisory only will produce warnings when your package is installed as a dependency.

vmalaga commented 7 years ago

@ggarnier of course that is better to reference to a specific version of node, but also, is possible the developer needs a feature apeared on >= x.x.x version so he only need to be sure his app run with a version from which this functionality came out. And of course, in this way there is also the danger that a functionality will be removed in later versions and the application will stop working.

But apart from what I can think, nodejs and npm support that nomenclature, also I make a try with nvm install >= 8.6.0 and works fine, so is not loggical not support it on tsuru

In addition tsuru only shows a very generic error and it is difficult to diagnose, it could show a warning message of the type

if [[ $NODE_VERSION =~ ^\>= ]]; then 
   echo "Node version ambigous, please dont use >= notation"
fi
ggarnier commented 7 years ago

I don't think it's worth the effort to try parsing different types of ranges (it could also be <, ~> and so on). We can show a specific error message, as you suggested.

The best solution for your problem would be creating a .nvmrc or .node-version file with a specific version in it. If one of these files exists, we use that to set the Node version. The version from engines.node in package.json is just a fallback in case you don't have .nvmrc or .node-version files.