maizzle / framework

Quickly build HTML emails with Tailwind CSS.
https://maizzle.com
MIT License
1.21k stars 48 forks source link

Error: ENOTEMPTY: directory not empty while saving template #914

Closed sidvishnoi closed 10 months ago

sidvishnoi commented 1 year ago

Updating a template file while running maizzle serve crashes the process. Just pressing Ctrl+S without even any actual changes is enough to crash the maizzle serve process.

config.js ```js const path = require('path'); const os = require('os'); /** @param {string[]} paths */ const p = (...paths) => path.join(__dirname, ...paths); module.exports = { tailwind: { config: require('./tailwind.config.js'), }, build: { browsersync: { port: 5050 }, layouts: { root: p('src/layouts'), tagName: 'layout' }, templates: { source: p('src/templates'), destination: { path: path.join(os.tmpdir(), 'maizzle-emails'), }, assets: { source: p('src/images'), destination: 'images' }, }, components: { root: p('src/components') }, fail: 'verbose', }, locals: { // add your locals here }, }; ```
Output of maizzle serve -nc ``` ✔ Re-built 2 templates in 0.227s [Browsersync] Access URLs: -------------------------------------- Local: http://localhost:5050 External: http://192.168.0.198:5050 -------------------------------------- UI: http://localhost:3001 UI External: http://localhost:3001 -------------------------------------- [Browsersync] Serving files from: /tmp/maizzle-emails ✔ Compiled in 0.152s [/home/redacted/src/templates/welcome.html] ✖ ENOTEMPTY: directory not empty, rmdir '/tmp/maizzle-emails' node:internal/process/promises:288 triggerUncaughtException(err, true /* fromPromise */); ^ [Error: ENOTEMPTY: directory not empty, rmdir '/tmp/maizzle-emails'] { errno: -39, code: 'ENOTEMPTY', syscall: 'rmdir', path: '/tmp/maizzle-emails' } ```
sidvishnoi commented 1 year ago

This seems to be triggered if the template contains a <component>. If I comment out the <component>, it reloads just fine. Though, rarely, it crashes without component too (that's likely some other issue).

cossssmin commented 1 year ago

Might also have to do with the tmpdir and browsersync, not sure. Can you provide a repo that reproduces the issue so we can test? Btw the tailwind config goes under build (docs).

sidvishnoi commented 1 year ago

Btw the tailwind config goes under build

Heh. I guess it worked as Maizzle looked for default config file.

Can you provide a repo that reproduces the issue so we can test?

https://github.com/sidvishnoi/repro-maizzle-914

sidvishnoi commented 1 year ago

Btw the tailwind config goes under build

I think https://maizzle.com/docs/api#options confused me.

{
  tailwind: { // tailwind listed separately here
    config: {},
    css: '',
    compiled: '',
  },
  maizzle: {}, // "The Maizzle Environment configuration object." Isn't `tailwind` part of `maizzle.build`?
  beforeRender() {},
  afterRender() {},
  afterTransformers() {},
}
cossssmin commented 1 year ago

I think it's because of that __dirname in your p() method, it results in destination.path containing invalid characters, thus it fails.

For example, your code gives me this destination path on Windows:

C:\\Users\\Cosmin\\AppData\\Local\\Temp\\maizzle-emails\\C:\\Users\\Cosmin\\Desktop\\repro-maizzle-914\\src\\templates\\welcome.html

Removing __dirname fixes it for me.

The maizzle serve command will properly throw an error starting with v4.4.0, right now it just gives you that warning but no error stack, which makes it hard to debug.

Also, note that you don't need to set the tailwind.config.js path under build if you already have the config file in the root of your project - Maizzle will pick it up automatically. This is only for cases where the config file is elsewhere, or it's named differently.

sidvishnoi commented 1 year ago

Removing __dirname does fix it for me too. But it's likely not due to invalid characters (on Windows yes the path is invalid, but on Linux, the formatted path was correct).

I replaced my p() function as following (which should result in correct path on both Windows and Linux):

/** @param {string} paths */
const p = (paths) => path.join(__dirname, ...paths.split('/'));

I think Maizzle isn't liking absolute paths here.

The maizzle serve command will properly throw an error starting with v4.4.0, right now it just gives you that warning but no error stack, which makes it hard to debud.

That would be very helpful. Thanks!

sidvishnoi commented 1 year ago

In particular, I updated build.templates.source to not use absolute path, and it worked fine.

I was able to pin it down to https://github.com/maizzle/framework/blob/4f25e8e20c9d78b81e9467b656cc63e33f1f2b0e/src/generators/output/to-disk.js#L37

Commenting out the line of course avoids the error. I'm not sure why this error is triggered. I thought maybe the fs-extra library is causing issues, so I tried replacing it with:

await require('fs/promises').rm(outputDir, {recursive: true,force:true})

But that didn't work either. What worked is adding a retry, which maybe indicates the directory was locked by some other task and so it failed to delete.

await require('fs/promises').rm(outputDir, {recursive: true,force:true,maxRetries: 1})
cossssmin commented 1 year ago

fs.remove(outputDir) isn't called when you're running the local dev server and saving changes to a template (i.e. file from src/templates in your project) - it runs only once, when you start the server (and there's no error at that time). However, when you save changes to something like a layout, a component, or a config file (config.js or tailwind.config.js), then everything gets rebuilt and that's when that method is called.

I think there's something else going on, some other (system?) process may be interfering for you - I never get ENOTEMPTY, but I do get the invalid path characters error (I explained what causes it). Keeping this open for now.

sidvishnoi commented 1 year ago

fs.remove(outputDir) isn't called when you're running the local dev server and saving changes to a template .... However, when you save changes to something like a layout, a component, or a config file (config.js or tailwind.config.js), then everything gets rebuilt and that's when that method is called.

I think the build.templates.source being absolute (via p()), while globalPaths being relative here causes it to be called even on template change (i.e. ignored doesn't work right), which leads to two watchers on same files (hence lock somehow; could be Ubuntu specific issue). https://github.com/maizzle/framework/blob/4f25e8e20c9d78b81e9467b656cc63e33f1f2b0e/src/commands/serve.js#L117

If I add awaitWriteFinish in watch options, it works fine.

but I do get the invalid path characters error (I explained what causes it).

Even with the updated p() from https://github.com/maizzle/framework/issues/914#issuecomment-1438161070?

Keeping this open for now.

Thank you. I'll investigate further as I get opportunity.


I've added workaround for my case by detecting some project-specific env vars, to not use absolute paths with maizzle serve. I needed absolute paths for other part of the project.

bravecrayon commented 1 year ago

I think I'm having this issue too and I've been able to recreate it fairly easily, but for me it only happens sometimes. Although, on this one project I'm working on, it feels like it happens every 4 or 5 saves. It appears that this only occurs when saving layouts/main.html.

  1. Start with the stock maizzle starter.
  2. maizzle serve -nc
  3. Open layouts/main.html.
  4. Update the preheader loop to a large array to slow it down, like: item in Array.from(Array(100000))
  5. Hold down CMD+S to hammer on it. Eventually it crashes.

Doing the same thing but instead on templates/transactional.html doesn't trigger it.

% maizzle serve -nc
✔ Re-built 2 templates in 0.911s
[Browsersync] Access URLs:
 --------------------------------------
       Local: http://localhost:3002
    External: http://192.168.0.100:3002
 --------------------------------------
          UI: http://localhost:3003
 UI External: http://localhost:3003
 --------------------------------------
[Browsersync] Serving files from: build_local
✔ Re-built 2 templates in 0.838s
⚠ EEXIST: file already exists, mkdir 'build_local'
✔ Re-built 0 templates in 0.359s
✔ Re-built 2 templates in 1.048s
✖ ENOTEMPTY: directory not empty, rmdir 'build_local'
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[Error: ENOTEMPTY: directory not empty, rmdir 'build_local'] {
  errno: -66,
  code: 'ENOTEMPTY',
  syscall: 'rmdir',
  path: 'build_local'
}
cossssmin commented 10 months ago

This should be fixed in today's 4.6.1 release, we've added some options to the file watcher so that it waits until files are written.