os-js / osjs-proc-provider

OS.js Process Service Provider
Other
4 stars 3 forks source link

Intermittent EIO error when using a PTY process #6

Open miladhashemi opened 3 years ago

miladhashemi commented 3 years ago

Hi, I wrote below code, and sometimes I have some issues:

const customProc = core.make('osjs/proc');
customProc.pty('ls -ltrh').then(p => {
      // Process events
      p.on('data', str => console.log(str));
      p.on('error', error => console.error(error)); // Spawn errors

      // Send data to the shell
      p.send('Hello World\n');

    });
  1. p.send() has output on console only when I login to Os.js
  2. Sometimes returns bellow error, Do U know what causes this error? Object { errno: "EIO", code: "EIO", syscall: "read" }
andersevenrud commented 3 years ago

p.send() has output on console only when I login to Os.js

You need a user session in order to use this module. This is for security reasons. You could override this to allow without a session, but I do not recommend it.

Sometimes returns bellow error, Do U know what causes this error?

An EIO code means that some operation is taking place (in this case read) after the socket has been closed. You can probably debug this by checking if .on('exit', () => console.log('EXITED')) is called before you .send() something.

Just a note on passing arguments to your commands: https://github.com/os-js/osjs-proc-provider#passing-arguments You have to do it like this: .pty('ls', '-ltrh').

miladhashemi commented 3 years ago

You can probably debug this by checking if .on('exit', () => console.log('EXITED')) is called before you .send() something.

I get this error without using send method. and Exited log is shown after error. It means before exiting process i get this error.

customProc.pty('ls').then(p => {
    // Process events
    p.on('data', str => console.log(str));
    p.on('exit', () => console.log('EXITED'));
    p.on('error', error => console.error(error)); // Spawn errors
    // Send data to the shell
    // p.send('Hello World\n');
    // You can kill long running processes
    // p.kill();
  });
andersevenrud commented 3 years ago

Can you show me an example where you're using this ?

mahsashadi commented 3 years ago

I have the same problem. I wrote an app, and I use osjs/proc to list folders, when the app is rendered. Most of the times it causes EIO error with error number 5

export default function App() {

    useEffect(() => {
        getFolderList();
      }, []);

    function getFolderList(){
        osjs.make('osjs/proc')
        .pty('ls' , '-d */')
        .then(p => {
            // Process events
            p.on('data', data =>{     
                   // do something 
            })
            p.on('exit', code => console.info('EXITED', code))
            // Internal events
            p.on('spawned', () => {}); // Spawn successful
            p.on('error', error => console.error(error)); // Spawn errors
            //p.send('Hello World\n');
            // You can kill long running processes
            p.kill();
          })
    }
andersevenrud commented 3 years ago

I think the problem here is that you immediately kill the PTY after it was created. This probably closes the socket before the data events are finished.

Use .exec(cmd, ...args) => Promise<{code, stdout, stderr}> instead of .pty(cmd, ...args) => Promise<p> instead if you just want to perform simple commands to get the output.

andersevenrud commented 3 years ago

I think the problem here is that you immediately kill the PTY after it was created. This probably closes the socket before the data events are finished.

Which means that if you remove the p.kill() this probably works as you expected. I still recommend using exec though in cases like this. A PTY is more suited to stream data and send input commands to a running process.

Also, expanding on what I said in an earlier comment about arguments, you should do this: '-d', '*/' instead of '-d */'. Whenever you have a whitespace (excluding strings) it should be a separate argument. Ex

command -a -b 1 -c "hello world" --bar

translates to: 'command', '-a', '-b', 1, '"hello world"', '--bar'

andersevenrud commented 3 years ago

// You can kill long running processes

I guess I should update the wording here. Might not be immediately obvious that method is meant for manually killing a process instead of waiting for it to close by itself or that the process does not close without a signal (like a command prompt or something with a loop in it).Hence the "long running" in that comment.

mahsashadi commented 3 years ago

Thanks a lot. using .exec(cmd, ...args) => Promise<{code, stdout, stderr}> completely solved my problem

andersevenrud commented 3 years ago

Great!

miladhashemi commented 3 years ago
customProc.pty('ls').then(p => {
    // Process events
    p.on('data', str => console.log(str));
    p.on('exit', () => console.log('EXITED'));
    p.on('error', error => console.error(error)); // Spawn errors
    // Send data to the shell
    // p.send('Hello World\n');
    // You can kill long running processes
    // p.kill();
  });

I tested this code on the file manager context menu.

andersevenrud commented 3 years ago

I tested this code on the file manager context menu.

And ?

miladhashemi commented 3 years ago

And ?

....and without killing process, sometimes I get EIO error, and I can't find out what is source of this error. I know ptyis used when we are using interactive sell or we want to stream... but this error isn't clear for me why some time it thrown.

andersevenrud commented 3 years ago

Since it's intermittent it has to be some kind of race condition on the server end. Do you still get the output data in this case ?

Also, try using .spawn() instead of .pty().

miladhashemi commented 3 years ago

Do you still get the output data in this case ?

Yes, almost always i get data as output.

Also, try using .spawn() instead of .pty().

I tested command with .spawn, .exec. But just in .pty i get this error.

andersevenrud commented 3 years ago

Yes, almost always i get data as output.

If it's not definitively always, then we do have some sort of racing condition going on here.

As I mentioned in an earlier comment, the EIO error is explained by this behavior.

So I guess I have to do some testing on my own to reproduce this.

Just to make sure I have the same setup, what node version are you running ?

miladhashemi commented 3 years ago

Just to make sure I have the same setup, what node version are you running ?

node v12.18.0

andersevenrud commented 3 years ago

I can confirm the same issue on latest node v14. So I'll look for a fix.

andersevenrud commented 3 years ago

Update: I only get this on PTY however. .spawn() works as expected.

miladhashemi commented 3 years ago

Update: I only get this on PTY however. .spawn() works as expected.

Yes, only .pty() is thrown this error. .spawn() and .exec() works fine.

andersevenrud commented 3 years ago

Oh. I now realize I read your comment wrong!

It seems that this is a problem with node-pty though and not something I really can fix on my end :thinking:

There's mention of a workaround, but it would have to be added by the developer using this feature, sadly.

I'll re-open this issue and do some deeper investegation when I have some more time to spare.

miladhashemi commented 3 years ago

but my host was ubuntu! 🤔

andersevenrud commented 3 years ago

The distro does not matter it seems. I get the same on Arch and Gentoo as well with the node-pty library.