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

Problems with running prod build #99

Closed
ItsInsajd opened this issue Jan 31, 2020 · 10 comments
Closed

Problems with running prod build #99

ItsInsajd opened this issue Jan 31, 2020 · 10 comments
Labels
bug Something isn't working
Milestone

Comments

@ItsInsajd
Copy link

Describe the bug
I wanted to check how reliable is to use nodejs blueprint to kickstart a project with azure deployment. Commands used for prod build do not work as excepted, but I may be doing something wrong, so I decided to ask here.

To Reproduce
Run blueprint as monolith, server only. Database probably doesn't matter, but i selected postgres. Then run npm install in main dir and in /server dir. Next:
cd server
npm run build

First problem:
After running npm run build, it logs an error when running copy-resources script:

[email protected] copy-resources C:\projects\hip-test\server
ts-node scripts/copy-resources.ts
cp: dest is not a directory (too many sources)`

After a bit of digging, it turns out an error lies in this part in copy-resources.js

const out = 'dist';
if (!fs.existsSync(out)) {
    fs.mkdirSync(out);
    fs.mkdirSync(out + '/config');
}

I think because dist folder already exists after build, /config folder cannot be created and files cannot be copied properly. Moving fs.mkdirSync(out + '/config'); outside of that if fixes this problem.

Second problem:
If then you try to use node disc/src/main.js to run the app, next problem arises.

internal/fs/utils.js:220
    throw err;
    ^

Error: ENOENT: no such file or directory, open 'C:\projects\hip-test\server\dist\src\config/application.yml'
    at Object.openSync (fs.js:440:3)
    at Object.readFileSync (fs.js:342:35)
    at Object.<anonymous> (C:\projects\hip-test\server\dist\src\config\config.js:88:50)
    at Module._compile (internal/modules/cjs/loader.js:955:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:991:10)
    at Module.load (internal/modules/cjs/loader.js:811:32)
    at Function.Module._load (internal/modules/cjs/loader.js:723:14)
    at Module.require (internal/modules/cjs/loader.js:848:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (C:\projects\hip-test\server\dist\src\client\header-util.js:3:18) {
  errno: -4058,
  syscall: 'open',
  code: 'ENOENT',
  path: 'C:\\projects\\hip-test\\server\\dist\\src\\config/application.yml'
}

If you inspect the path where application.yml file should be, you'll notice it tries to find it inside dist/src/config/ file. But this file is inside dist/scripts, so this path is wrong. I fixed it by changing paths in config file:

const yamlConfig = yaml.safeLoad(fs.readFileSync(__dirname + '/../../config/application.yml', 'utf8'));
const envYamlConfig = yaml.safeLoad(fs.readFileSync(__dirname + `/../../config/application-${process.env.NODE_ENV}.yml`, 'utf8'));

Third problem:
For some reason, env variables are not resolved on azure. For example, process.env.NODE_ENV is undefined and i had to hardcode 'prod' in this line

const envYamlConfig = yaml.safeLoad(fs.readFileSync(__dirname + '/../../config/application-prod.yml', 'utf8'));

to make it work. NODE_ENV should come from .env file, but it's not resolved.

Expected behavior
App builds and starts as expected.

Desktop (please complete the following information):

  • OS: Windows 10
  • Node: 12.14.1
  • npm: 6.13.7

Additional context
After one day of work, we managed to finally run it on azure, but env variables are in some parts hardcoded, which is far from ideal.

@ghost ghost self-assigned this Jan 31, 2020
@ghost ghost added the bug Something isn't working label Jan 31, 2020
@ghost
Copy link

ghost commented Jan 31, 2020

Hi @ItsInsajd ,
thanks for the feedback, it is a great job that you have performed.
This is a beta version yet, I have tested a lot of use cases, so it is very useful to receive feedbacks.
So, let's on the analysis:

  1. The if code:
const out = 'dist';
if (!fs.existsSync(out)) {
    fs.mkdirSync(out);
    fs.mkdirSync(out + '/config');
}

I understand now that it fails in case that the dist folder exist, but it does not contains config folder... So we have to check if exist dist/config path and create it.

  1. To run directly javascript build app, you have to execute:
node dist/main.js

Or

 npm run start:prod

Please check the README generated under server part

  1. If you choose to run with node dist/main.js, maybe the config path is not more calculated well:
const yamlConfig = yaml.safeLoad(fs.readFileSync(__dirname + '/../../config/application.yml', 'utf8'));
const envYamlConfig = yaml.safeLoad(fs.readFileSync(__dirname + `/../../config/application-${process.env.NODE_ENV}.yml`, 'utf8'));

So I have to fix that scenario.

For the env variable, I notice that in some machine they are not read.
However, you can execute a npm/node task passing it. For example:

# run production build with node
$ set NODE_ENV=prod&& node dist/main.js

You can always check it in generated server README

@ItsInsajd
Copy link
Author

ItsInsajd commented Jan 31, 2020

Hey, thanks for response. In my case, node dist/main.js or npm run start:prod won't work, because there is no main.js file in dist folder. When running npm run build, my dist folder is as follows:

dist/
----config/
----scripts/
----src/

So running node dist/main.js will fail.
I also tried reverting copy-resource.ts to default state (before my change described in first post). Running npm run build then shows the same error as earlier: cp: dest is not a directory (too many sources).
So I modified copy-resources.ts again. Calling set NODE_ENV=prod&& npm run build && npm run start:prod causes copy-resource script to run two times, so I decided to check if config folder exists to avoid another error (Error: EEXIST: file already exists, mkdir 'dist/config').

const out = 'dist';
if (!fs.existsSync(out)) {
  fs.mkdirSync(out);
}
if (!fs.existsSync(out + '/config')) {
  fs.mkdirSync(out + '/config');
}

Buuuuut this still fails, because there is no main.js in dist/. In my case, main.js lies inside dist/src/. And the same goes for node dist/main.js.

@ghost
Copy link

ghost commented Jan 31, 2020

Hi,
try these steps:

  • Create dist/config empty folder under server folder
  • Run set NODE_ENV=prod&& npm run build && npm run start:prod

After this you can run npm run start:prod because you have the javascript build.
You can refer to the README. generated. The start:prod task is not allowed without npm build.

Besides, for the env variables the generated project uses those defined in the .env file. Maybe in Azure or in the cloud that is ignored and you have to pass in the npm task (set NODE_ENV=prod&& npm ... ). So the env variables are not in clear.

However, I have to fix:

  • The error in the copy-resources.js to copy, generate and read config.
  • The prod running using application.yml profile for environment, I will add a control to check the correct env used
  • Verify correct yaml path in dev and prod running.

Thanks a lot for the feedbacks.

@ItsInsajd
Copy link
Author

Hey, to clarify, I ran npm run build before npm run start:prod. Creating /scripts folder inside dist/ also doesn't seem to make any difference. The main problem is that main.js file is not in dist folder, it's inside dist/src/, where other built source files are. I don't know why this is the case. I ran set NODE_ENV=prod && npm run build && npm run start:prod on my friend's PC with the same setup as described in first post and the result is the same as mine.

If i understand you correctly, after build the main.js file should be in dist/ path? Can you show me screenshot of your dist/ directory?

Anyways, it seems to work with node dist/src/main.js so this is not the biggest problem. I will try to dig a bit more if I have time for it, maybe running on my home laptop, where I have different configuration. For now, I have no clue what's wrong.

Thanks for your time :)

@ghost
Copy link

ghost commented Jan 31, 2020

@ItsInsajd you are welcome! feel free to say what you want :)
So, let me explain what should be the normale behaviour:

  • You should have a dist/config folder, not a dist/scripts. Scripts folder is not copied.
  • It's strange that the main.js is under dist/src.. I will investigate on it.
    I will work for a fix, and if you want you will test it and give me the ok ;)
    Thanks a lot

@ghost
Copy link

ghost commented Feb 3, 2020

I'm checking on resolve-env branch

@ItsInsajd , you can dowload that branch and check if is all ok?

Follow the steps: https://github.com/jhipster/generator-jhipster-nodejs/tree/bug/resolve-env#-steps-to-develop-a-generator-feature-and-test-it

However, the right paths, that I have fixed, of the main build under server is dist/main.js . I have added also integration test for this: https://github.com/jhipster/generator-jhipster-nodejs/runs/423982763

Besides I'm adding a feature with webpack for a full bundle.js that not require node_modules folder.

@ghost ghost added this to To do in nodejs blueprint via automation Feb 3, 2020
@ghost ghost moved this from To do to In progress in nodejs blueprint Feb 3, 2020
@ghost ghost added this to the 1.0.0-beta.3 milestone Feb 4, 2020
@ghost
Copy link

ghost commented Feb 4, 2020

Fix with #102 , I will release the 1.0.0-beta.3

@ghost
Copy link

ghost commented Feb 4, 2020

Hi @ItsInsajd,
you can test the new version just published 1.0.0-beta.3. Feel free to open new issues if there are another problems. Thanks!

@ghost ghost closed this as completed Feb 4, 2020
nodejs blueprint automation moved this from In progress to Done Feb 4, 2020
@ItsInsajd
Copy link
Author

Hey, sorry for late update, but it seems that indeed problems listed in this issue have been resolved. I can now run set NODE_ENV=prod&& npm run build && npm run start:prod and the application builds and runs as expected. Thanks for your work!

@ghost
Copy link

ghost commented Feb 5, 2020

Hi @ItsInsajd , no problem for the delay.. you are welcome!
I'ts my pleasure to grow up the community around this project. Thanks you a lot!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

1 participant