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

Assets missing + nondeterministic build (probable regression?) #2018

Open
2 of 4 tasks
SmallhillCZ opened this issue Apr 5, 2023 · 6 comments
Open
2 of 4 tasks

Assets missing + nondeterministic build (probable regression?) #2018

SmallhillCZ opened this issue Apr 5, 2023 · 6 comments

Comments

@SmallhillCZ
Copy link

SmallhillCZ commented Apr 5, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

This is a probable regression from already closed issues (see below). When machine is slow or slowed down, sometimes not all assets will get copied. This creates non deterministicly faulty outputs for NestJS builds.

This means when you are for example building Docker image using BuildKit, which has parallel execution, a CPU and HDD intensive task of another Docker build step will cause part of your assets not being copied.

As a direct consequence part of your application will not be working as usual. Also as this not being a code part, it will probably not fail during startup and you will learn the hard way, e.g. users not being able to register, because the HTML file with validation email cannot be loaded.

This was discussed previously in #1828 and #749 and it was presumably caused by the timeout in assets manager on L26. Also the note on L25 actually suggests this could happen for large files.

public closeWatchers() {
// Consider adjusting this for larger files
const timeoutMs = 500;
const closeFn = () => {
if (this.actionInProgress) {
this.actionInProgress = false;
setTimeout(closeFn, timeoutMs);
} else {
this.watchers.forEach((watcher) => watcher.close());
}
};
setTimeout(closeFn, timeoutMs);
}

Minimum reproduction code

The issue seems the same as before and the reproduction code will be hard to create, as it is a concurrency issue with other processes. So would like to discuss if it's needed before coding.

Steps to reproduce

  1. Run NestJS build with either
    • lot of files
    • slow machine
    • machine slowed down with other parallel processes using HDD
  2. Random part of your assets will not make it to the build

Expected behavior

All assets must be in the build no matter the build time.

Package version

9.3.0

NestJS version

9.4.0

Node.js version

18.14.2

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

At least the timeoutMs should be configurable in nest-cli.json or somewhere, but I think by default the copy process should never be killed before finishing.

@SmallhillCZ SmallhillCZ changed the title Build cannot be run deterministically because of assets timeout (probable regression?) Assets missing + nondeterministic build because of assets timeout (probable regression?) Apr 5, 2023
@SmallhillCZ SmallhillCZ changed the title Assets missing + nondeterministic build because of assets timeout (probable regression?) Assets missing + nondeterministic build (probable regression?) Apr 5, 2023
@SmallhillCZ
Copy link
Author

SmallhillCZ commented Apr 5, 2023

Looking at the code it looks weird, that the variable which is there to "avoid watches getting cutoff" here:

// Set action to true to avoid watches getting cutoff
this.actionInProgress = true;

is set to false to cut the watchers off after timeout anyway here:

if (this.actionInProgress) {
this.actionInProgress = false;
setTimeout(closeFn, timeoutMs);
} else {
this.watchers.forEach((watcher) => watcher.close());
}

@natoszme
Copy link

Hi there! Same thing happening here: sometimes in my deploys in aws beanstalk, the assets folder is missing in one of the instances. Never happened in my local environment, so could not reproduce it yet.
But it did happen several times since I started using nest :(

Nest version: 9.2.0
Node version: 14.18.0

@natoszme
Copy link

Hi everyone! I've been struggling with this bug several times since my last post, so I fixed it ensuring that every asset is copied to the production dist folder. To to it, I made a small change to my package.json deploy command:

Before: "deploy": "npm install && npm run build && npm run start:prod"
After: "deploy": "npm install && npm run build && npm run ensureBuild && npm run start:prod"

Where:

  • "ensureBuild": "make copyAssets",
  • and copyAssets is defined in a Makefile as:
assetsDirectory = notifications/templates //change it for yours
distDirectory = dist/${assetsDirectory}
srcDirectory = src/${assetsDirectory}

copyAssets:
	rm -rf $(distDirectory)
	mkdir $(distDirectory)
	cp -r $(srcDirectory)/. $(distDirectory)/

It's been a couple of weeks since I deployed it and had no problems since then. Hope it helps!

@simonbrunel
Copy link

We are experiencing the same, making deployment very unreliable. It happens on .hbs files (handlebar templates), which are dynamically loaded on demand, meaning that the server correctly starts and then randomly crashes when trying to access these assets. It's also very hard to reproduce (it only happens when built on CI), so I can't provide a reproduction repo.

@kamilmysliwiec is there anything that come to your mind that would help fixing / workaround this issue?

@artpumpkin
Copy link

Same here with .proto files.

@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

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

No branches or pull requests

5 participants