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

unexpected behavior: after awaiting esbuild.build, stdout hasn't received all output #1869

Closed
mreinstein opened this issue Dec 19, 2021 · 14 comments

Comments

@mreinstein
Copy link

mreinstein commented Dec 19, 2021

I'm integrating esbuild into a build process on our backend, and there was something peculiar I noticed.

After awaiting the esbuild completion, stdout may not have all of the output:

mybuild.js:

const cfg = {
   bundle: true,
   minify: true,
   allowOverwrite: true,
   logLevel: 'silent',}

await esbuild.build(cfg)

// at this point I'd expect stdout to have received the entire bundle string but the output gets cutoff

process.exit(0)

It wasn't what I expected. I know process.exit() is an anti-pattern, but I'm demonstrating that if I just exit the script after that await, I would have though everything would be in stdout by then. It's resulting in truncation when I do:

node mybuild.js > out.js

Is this a bug, or a known behavior?

@evanw
Copy link
Owner

evanw commented Dec 20, 2021

No, it's not known behavior. I could potentially try to figure out why it's happening if you're able to provide a way to reproduce the issue. I believe the data written to stdout happens in a single console.log() call here:

if (response.writeToStdout !== void 0) console.log(protocol.decodeUTF8(response!.writeToStdout).replace(/\n$/, ''));

@mreinstein
Copy link
Author

In looking at the notes for node.js on process.stdout (the underlying stream that console.log writes to) it can be asynchronous in some cases: https://nodejs.org/dist/latest-v16.x/docs/api/process.html#a-note-on-process-io

I think what's happening is the code is running under one of these cases where console.log is asynchronous, and with a large enough bundle on a device with constrained i/o (most cloud providers) stdout isn't fully written to before process.exit() is called.

Which sounds very similar to what happened here: nodejs/node#7743

I don't know what (if any) the remediation steps are. My first instinct was trying to attach some events to stdout to determine when the write finishes, but I don't know if that's feasible.

@mreinstein
Copy link
Author

mreinstein commented Dec 21, 2021

if there is no remediation step code-wise, maybe we put some kind of blurb in the docs around the mode where we write to stdout ? I would imagine that anyone invoking esbuild from a cli from any of the envs where console.log is async could be affected by this, but I dunno how common that situation is/will be. It really sucks because it was painful to track this down.

@J-Cake
Copy link

J-Cake commented Dec 24, 2021

when in doubt: setTimeout(() => process.exit(), 1000)

@mreinstein
Copy link
Author

mreinstein commented Dec 24, 2021

That doesn't solve the problem. This issue isn't about process.exit() specifically, it's the fact that after

await esbuild.build(...)

finishes, the expectation of the API is that all of the content has finished writing to stdout. If there are other logic or code that is expecting that to have finished when the await is finished it may break things.

@J-Cake
Copy link

J-Cake commented Dec 25, 2021

  1. Please don't take my comment seriously. It was meant as a joke. I understand what you're saying.
  2. How about this as a temporary workaround:
await new Promise(resolve => { esbuild.build( ... ); process.stdout.once('drain', () => resolve());  });

I have no way of testing at the moment, so let me know if that's totally unhelpful

@mreinstein
Copy link
Author

Please don't take my comment seriously. It was meant as a joke

Heh, sorry I wasn't sure. Lack of tone on the internet strikes again. :)

How about this as a temporary workaround:

That looks like it could work. I forget the behavior of drain in node stream contexts but it looks promising, thanks! 👍

@J-Cake
Copy link

J-Cake commented Dec 26, 2021

If I've understood correctly, the drain event only gets invoked once the stream's buffer exceeds a defined size threshold and has been completely cleared, meaning it's not a reliable way to detect completion, but if you hook a timeout on it, then it should increase the efficacy somewhat.

@evanw
Copy link
Owner

evanw commented Apr 5, 2022

I'm going to mark this as unactionable since a way to reproduce the issue has not been provided in this thread, so there's no way to verify a fix.

@mreinstein
Copy link
Author

Yeah I think that's fair. It's technically actionable I think but it's not easy to setup. It would require having a hosted virtual instance with extremely low i/o, running windows as an OS.

I'm glad this is being labeled rather than closed. It's a pretty bad issue when encountered because there's no error, and nothing obviously wrong. Keeping it open will make it easier to find should others search for this later.

@J-Cake
Copy link

J-Cake commented Apr 7, 2022

Just to quench my curiosity, how did you work around this?

@mreinstein
Copy link
Author

how did you work around this?

I had code that basically looked like this:

await esbuild.build(cfg)

process.exit()

I worked around this by simply removing process.exit() This was ok in my situation because the esbuild step was the very last thing the process needed to do. My workaround would not have worked if my code did something more like this:

await esbuild.build(cfg)

doMoreStuffThatDependsOnTheBuildBeingComplete()

@J-Cake
Copy link

J-Cake commented Apr 7, 2022

ah I see. That's a friendly solution. Otherwise, I guess you could use the shell to spawn a new node instance when that one finishes, should you need to do more stuff.

@evanw
Copy link
Owner

evanw commented Dec 4, 2022

I'm going to close this issue since it has been unactionable for a long time without any progress. If someone is willing to investigate this and discover the cause (and maybe even propose a fix), then we can revive 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

3 participants