smogon / pokemon-showdown

Pokémon battle simulator.
https://pokemonshowdown.com
MIT License
4.77k stars 2.79k forks source link

Error event is emitted when EOF sent to `./pokemon-showdown simulate-battle` #5403

Closed yuzeh closed 5 years ago

yuzeh commented 5 years ago

Doesn't look to actually be harmful, but I don't remember this being there in the past.

Error [ERR_STDOUT_CLOSE]: process.stdout cannot be closed
    at WriteStream.stdout._destroy (internal/process/stdio.js:23:18)
    at WriteStream.destroy (internal/streams/destroy.js:32:8)
    at WriteStream.Socket._final (net.js:350:17)
    at callFinal (_stream_writable.js:612:10)
    at process._tickCallback (internal/process/next_tick.js:63:19)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
Zarel commented 5 years ago

Hm. I guess the best fix here is probably to make closing the stdout stream a no-op. It should definitely be possible to pipe to stdout without getting this error.

yuzeh commented 5 years ago

My workaround for this is to send SIGINT (^C) to the process when I need to shut it down.

Zarel commented 5 years ago

Anyone else have opinions? @scheibo?

scheibo commented 5 years ago

Do you have simple repro steps? pokemon-showdown simulate-battle and then Ctrl+D doesn't cause issues for me?

Zarel commented 5 years ago

It might be OS-specific. Whether or not stdout is closable sounds like the sort of thing that would be OS-specific.

yuzeh commented 5 years ago

Do you have simple repro steps? pokemon-showdown simulate-battle and then Ctrl+D doesn't cause issues for me?

This happens to me on OSX and Linux.

scheibo commented 5 years ago

That's what I'm doing it on as well :). Does 'pokemon-showdown simulate-battle and then Ctrl+D immediately after' really break for you, or what steps are you doing?

yuzeh commented 5 years ago

Yeah, ^D right after starting the program breaks. It also breaks if you've sent a few lines through stdin already.

Here's a few examples:

(pkmn) ➜  Pokemon-Showdown git:(master) ./pokemon-showdown simulate-battle
events.js:167
      throw er; // Unhandled 'error' event
      ^

Error [ERR_STDOUT_CLOSE]: process.stdout cannot be closed
    at WriteStream.stdout._destroy (internal/process/stdio.js:23:18)
    at WriteStream.destroy (internal/streams/destroy.js:32:8)
    at WriteStream.Socket._final (net.js:350:17)
    at callFinal (_stream_writable.js:612:10)
    at process._tickCallback (internal/process/next_tick.js:63:19)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
(pkmn) ➜  Pokemon-Showdown git:(master) ./pokemon-showdown simulate-battle
>start {"formatid":"gen7randombattle","seed":[50234,8145,47152,37130],"rated":"Tournament battle"}
>player p1 {"name":"lala"}
update
|player|p1|lala|

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

Error [ERR_STDOUT_CLOSE]: process.stdout cannot be closed
    at WriteStream.stdout._destroy (internal/process/stdio.js:23:18)
    at WriteStream.destroy (internal/streams/destroy.js:32:8)
    at WriteStream.Socket._final (net.js:350:17)
    at callFinal (_stream_writable.js:612:10)
    at process._tickCallback (internal/process/next_tick.js:63:19)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
scheibo commented 5 years ago
kjs@scheibo(toJSON)$ ./pokemon-showdown simulate-battle
>start {"formatid":"gen7randombattle","seed":[50234,8145,47152,37130],"rated":"Tournament battle"}
>player p1 {"name":"lala"}
update
|player|p1|lala|

kjs@scheibo(toJSON)$ echo $?
0

Can you maybe include uname -a output? @Zarel - can you repro on your machine(s)? I don't doubt that this is an issue, I just will have trouble fixing it if I can't repro :(

yuzeh commented 5 years ago

If it helps, I'm on node v10.4.1 on my mac and node v9.11.1 on the linux docker instance.

(pkmn) ➜  metagrok git:(master) ✗ uname -a
Darwin Dans-MacBook-Pro.local 18.2.0 Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RELEASE_X86_64 x86_64 i386 MacBookPro11,3 Darwin

[root@3fce702614b4 ~]# uname -a
Linux 3fce702614b4 4.9.125-linuxkit #1 SMP Fri Sep 7 08:20:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
scheibo commented 5 years ago

FWIW, we support node 10+ according to the README, and I bet most of us are actually on 11+. I'll try switching to an earlier version though, thanks for the details.

scheibo commented 5 years ago

Hey, would you look at that!

kjs@scheibo(master)$ nvm install 9.11.1
Downloading and installing node v9.11.1...
Downloading https://nodejs.org/dist/v9.11.1/node-v9.11.1-linux-x64.tar.xz...
##################################################################################################################################### 100.0%
Computing checksum with sha256sum
Checksums matched!
Now using node v9.11.1 (npm v5.6.0)
kjs@scheibo(master)$ node --version
v9.11.1
kjs@scheibo(master)$ ./pokemon-showdown simulate-battle
events.js:165
      throw er; // Unhandled 'error' event
      ^

Error [ERR_STDOUT_CLOSE]: process.stdout cannot be closed
    at WriteStream.stdout._destroy (internal/process/stdio.js:18:18)
    at WriteStream.destroy (internal/streams/destroy.js:32:8)
    at WriteStream.onSocketFinish (net.js:311:17)
    at WriteStream.emit (events.js:180:13)
    at finishMaybe (_stream_writable.js:636:14)
    at endWritable (_stream_writable.js:644:3)
    at WriteStream.Writable.end (_stream_writable.js:586:5)
    at WriteStream.Socket.end (net.js:485:31)
    at resolve (/usr/local/google/home/kjs/src/pkmn.cc/Pokemon-Showdown/.lib-dist/streams.js:371:30)
    at new Promise (<anonymous>)
Emitted 'error' event at:
    at emitErrorNT (internal/streams/destroy.js:64:8)
    at process._tickCallback (internal/process/next_tick.js:178:19)
scheibo commented 5 years ago

I tried all the nodes between 10.4 and 11, and it looks like this was fixed between 10.11 and 10.12. But since we supposedly support v10+ it should still be fixed.

scheibo commented 5 years ago

or maybe WriteStream, when wrapping stdout, should automatically suppress EOF

will look into doing that