fix EAGAIN error for stdin

[seblj]

I got some EAGAIN errors and tracked it down to being a problem of using fs.readFileSync or fs.readSync. It's really hard to reproduce, but it seems like others also have had this problem:

tatethurston/TwirpScript#208
nodejs/help#2663
https://stackoverflow.com/questions/40362369/stdin-read-fails-on-some-input

This seems to fix the problem.

I am not sure if it's the best way to do it, but it seems to work for me at least.

[nene]

Thanks for the PR, but I don't have enough details to evaluate this.

What is the actual problem that this PR is fixing. Like, how to reproduce the bug? I also haven't head from anybody else complaining about any issue that looks similar. Or at least I didn't manage to find anything in the issue tracker.

...after some more reading I think I sort of understand the problem, but I'm unsure about the fix. Unless I have a way to reproduce it, I really can't accept a fix on faith alone.

The fix you have included is pretty elaborate. But it seems to be a copy-paste from stack overflow post, which itself has received only a single upvote, so I'm not too keen to trust that. Given that reading stdin is such a common operation, I'm especially doubtful that one would need so much code to handle such a common task correctly.

[seblj]

Yes I understand. I only noticed this issue when combining it with a plugin I have for neovim (https://github.com/seblj/nvim-formatter), and when formatting a lot of files at once.

I will try to come up with an example to make it easy to reproduce without formatting a lot of files and without the plugin.

I just made the PR as an example for now and to make you aware that there is an issue with readFileSync and stdin

[nene]

Thanks. Your efforts are much appreciated. :)

[seblj]

I just pushed a much easier fix that still seems to work as expected!

This is the same way as how prettierd reads stdin:

https://github.com/fsouza/prettierd/blob/a9e110ad7a6e7ff91d80a0b33a2c82738fdefb56/src/prettierd.ts#L12
https://github.com/fsouza/prettierd/blob/a9e110ad7a6e7ff91d80a0b33a2c82738fdefb56/src/prettierd.ts#L125

I can still work on finding a reproducible example where it fails with EAGAIN if you want, but the problem is basically reading synchronously from process.stdin.fd which is in non-blocking mode: nodejs/node-v0.x-archive#8297

So if I understand it correctly, one should not read synchronously from process.stdin.fd, and since I wrap it in promisify and await the result, it works as expected.

Prettier for example uses this library: https://www.npmjs.com/package/get-stdin

https://github.com/prettier/prettier/blob/79d8a3983842d1d8a077f177ed8b983c1c596088/src/common/mockable.js#L4C1-L4C1

[seblj]

Okay I have a reproducible where I expect that it would format it correctly. However there is two different errors now with the fix I have, and what is currently the way it is handled. Using the get-stdin package like prettier fixes this problem though.

Create a file foo.sql

        CREATE TABLE
  users (
    id uuid PRIMARY KEY DEFAULT uuid_generate_v4 (),
                    email VARCHAR(50) UNIQUE NOT NULL,
    name VARCHAR(150) NOT NULL
  );

Run:

awk '{print $0; system("sleep .1");}' foo.sql | sql-formatter -l postgresql

This will give you the error of Error: no file specified and no data in stdin which is the EAGAIN error.

However, the fix I have produced currently will give another error since it will try to format only the first line since it will not wait until it's finished receiving input from stdin.

I will switch to using the get-stdin package like prettier in an upcoming commit where you can try the same reproducible, and you will se that it will then format it correctly

UPDATE:

In the latest commit, it should correctly format foo.sql. I did however have some issues with the import of the get-stdin package as it was not compatible with require(), so for now I just copy-pasted the library implementation for you to see that it works correctly.

[bitst0rm]

I got the same Error: no file specified and no data in stdin when formatting large database with more than 900 lines.
This patch does solve this issue.

[nene]

Thanks for the fix @seblj

For some reason Github failed to notify me about the comments and improvements you did two weeks ago. But thanks to @bitst0rm this PR got back to my attention.

I sort-of made the get-stdin library work by simply importing 8.0.0 version (not the latest 9.0.0). Tried a few things to get it to work with the ESM thingie, but then decided to not waste my time and gave up. Doesn't really matter much for this one-function library.

[seblj]

No problem! Thanks for merging!

[nene]

Released the fix as 12.2.4