jhipster / generator-jhipster-nodejs

A NodeJS blueprint that creates the backend using NestJS
https://www.npmjs.com/package/generator-jhipster-nodejs
Apache License 2.0
256 stars 79 forks source link

Problems with running prod build #99

Closed ItsInsajd closed 4 years ago

ItsInsajd commented 4 years ago

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:

hiptest@0.0.1 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):

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 commented 4 years ago

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.

  2. 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 commented 4 years ago

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 commented 4 years ago

Hi, try these steps:

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:

Thanks a lot for the feedbacks.

ItsInsajd commented 4 years ago

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 commented 4 years ago

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

ghost commented 4 years ago

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 commented 4 years ago

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

ghost commented 4 years ago

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!

ItsInsajd commented 4 years ago

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 commented 4 years ago

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!