lostintangent / node-azure

Tutorials that illustrate how to build Node.js apps with VS Code, and deploy them to Azure
http://azure.com/node
10 stars 7 forks source link

Walkthru of local git deployment to App Service results in deployment failure and 403 forbidden error #26

Closed nicolehaugen closed 7 years ago

nicolehaugen commented 7 years ago

The steps described in this walkthru result in a deployment error occurring - specifically the issue is in step #5 where the version of node is changed. If you leave the version of node left set to the default, the app successfully deploys.

This is likely a bug in the deployment script, but more follow up is needed to confirm this. There is also a question of whether the NPM version should be configurable.

Refer to this email for further details.

DeploymentError.docx.docx

lostintangent commented 7 years ago

@nicolehaugen79 Thanks for reporting this! I just sent a mail to the App Service PM (with you on it), so we can hopefully track this issue down. In the meantime, I wonder whether we shouldn't update that walkthrough to remove step #5? If it makes the demo work, that's awesome, since it also simplifies it as well :)

Regarding the question about allowing the NPM version o be configurable. That may be valuable, but I'd prefer not to expect/require users to do it, since the default NPM version that comes along with a specific Node.js version has been tested for compatibility. My hunch here is that the deployment scripts don't handle requests for Node versions that aren't actually available. But we'll see what details come out.

nicolehaugen commented 7 years ago

Thanks for starting the thread with the PM - I have logged this issue so that we can track the bug with node version 6.9.3 not currently being supported by the deploy script.

And then also logged this issue that tracks the 403 Forbidden error that occurs if the user fails to set the default node version (this only happens for Linux when using the Azure CLI): https://github.com/Azure-App-Service/kudu/issues/11

As for NPM, I'll wait on that one since I'm not sure whether there is truly user need for that.

lostintangent commented 7 years ago

This has been fixed!

RoboBurned commented 6 years ago

The issue still exists with node 8.1