(indev branch) Voice stopPlaying() seems to crash

[nicholastay]

So on the indev branch, when I try to do voiceConnection.stopPlaying() it crashes with a Error: write EPIPE on my computer - Windows 10 x64, node.js v0.12.9. However this only happens with the new 'fix' that was implemented in the indev branch to kill the ffmpeg/avconv processes when they were done. But this 'fix' only broke my ability to use the stopPlaying(). Upon commenting out this line it fixes the issue, but as you know it can leave ffmpeg running as a child process.

I'm still trying to find a fix for it, I'll keep this posted if I do find one. I suspect it may be because of the SIGINT being sent that is not handled properly on Windows? Just commenting out that line still crashes it, I have to comment out the whole block to allow it to run normally. That's not good as ffmpeg.exe does keep running in the background.

The line(s) causing it in the new indev update is here - https://github.com/hydrabolt/discord.js/blob/indev/lib/Voice/VoiceConnection.js#L94-L98

The full error stack I encountered is below:

Error: write EPIPE
  at exports._errnoException (util.js:746:11)
  at Socket._writeGeneric (net.js:692:26)
  at Socket._write (net.js:711:8)
  at doWrite (_stream_writable.js:301:12)
  at writeOrBuffer (_stream_writable.js:288:5)
  at Socket.Writable.write (_stream_writable.js:217:11)
  at Socket.write (net.js:636:40)
  at ReadStream.ondata (_stream_readable.js:540:20)
  at ReadStream.emit (events.js:107:17)
  at ReadStream.Readable.read (_stream_readable.js:373:10)
  at flow (_stream_readable.js:750:26)
  at Socket.<anonymous> (_stream_readable.js:607:7)
  at Socket.emit (events.js:104:17)
  at onwriteDrain (_stream_writable.js:371:12)
  at afterWrite (_stream_writable.js:359:5)
  at onwrite (_stream_writable.js:352:7)
  at Socket.WritableState.onwrite (_stream_writable.js:105:5)
  at WriteWrap.afterWrite (net.js:789:12)

Update: It also happens when I run it on an Ubuntu VM

[TehSeph]

Popping this here just as an idea:

Since the issue here is with Windows not understanding term signals and crashing, what about checking process.platform to see if the bot is running on Windows and, if it is, just kill ffmpeg with taskmanager:

if (this.streamProc) {
    this.streamProc.stdin.end();
    if (process.platform === 'win32') {
        let spawn = require('child_process').spawn;    
        spawn("taskkill", ["/pid", this.streamProc.pid, '/f', '/t']);
    } else {
        this.streamProc.kill();
    }
}

I also changed stdin.pause() to stdin.end() since that seemed like the more proper thing to use, and I've removed kill('SIGINT') since it shouldn't be needed and handled automatically by kill().

This is totally untested and just an idea. I have no clue if there are any repercussions of doing it this way, though I wouldn't think there are since it's essentially the Windows way of doing kill('SIGKILL') and forcing an immediate quit and any issues with doing this should be the same for every OS.

[nicholastay]

Actually, the same issue occurs on an Ubuntu virtual machine when I tested
it so it isn't specially a Windows issue, which I find very peculiar.
On Dec 22, 2015 4:21 AM, "Seph D. Warwick" notifications@github.com wrote:

Popping this here just as an idea:

Since the issue here is with Windows not understanding term signals and
crashing, what about checking process.platform to see if the bot is
running on Windows and if it is, just kill ffmpeg with taskmanager:

if (this.streamProc) {
this.streamProc.stdin.end();
if (process.platform === 'win32') {
let spawn = require('child_process').spawn;
spawn("taskkill", ["/pid", this.streamProc.pid, '/f', '/t']);
} else {
this.streamProc.kill();
}
}

I also changed stdin.pause() to stdin.end() since that seemed like the
more proper thing to use, and I've removed kill('SIGINT') since it
shouldn't be needed and handled automatically by kill().

This is totally untested and just an idea. I have no clue if there are any
repercussions of doing it this way, though I wouldn't think there are since
it's essentially the Windows way of doing kill('SIGKILL') and forcing an
immediate quit and any issues with doing this should be the same for every
OS.

—
Reply to this email directly or view it on GitHub
#98 (comment)
.

[meew0]

I'm getting a slightly different stacktrace:

events.js:142
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at exports._errnoException (util.js:856:11)
    at Pipe.onread (net.js:545:26)

But otherwise the same behaviour, stopPlaying consistently crashes.

[nicholastay]

I believe that this is actually the same error, but on different node
versions(?). I'm on v0.12.9 and so it gives that stacktrace. I think on
v4.x.x gives the stacktrace that you (@meew0) provided.
On Dec 24, 2015 7:33 AM, "meew0" notifications@github.com wrote:

I'm getting a slightly different stacktrace:

events.js:142
throw er; // Unhandled 'error' event
^

Error: read ECONNRESET
at exports._errnoException (util.js:856:11)
at Pipe.onread (net.js:545:26)

But otherwise the same behaviour, stopPlaying consistently crashes.

—
Reply to this email directly or view it on GitHub
#98 (comment)
.

[amishshah]

Fixed in 0113202 with the help of @qeled, streams are now unpiped before being destroyed