node-minecraft-protocol icon indicating copy to clipboard operation
node-minecraft-protocol copied to clipboard

Missing ping timeout callback when server is running but not responding to pings

Open deathcap opened this issue 9 years ago • 6 comments

ping.js does not appear to always call its callback method, it will be called with an error when a) the port is closed (example: options {host:'github.com', port:25565}), b) when a protocol error occurs (example: {host:'github.com', port:80}), but not c) when the server is running but does not acknowledge the pings.

To test this with a server, from https://github.com/PrismarineJS/node-minecraft-protocol/issues/327#issuecomment-174310946 made this modification

diff --git a/src/createServer.js b/src/createServer.js
index 5b56fdd..ef2b84e 100644
--- a/src/createServer.js
+++ b/src/createServer.js
@@ -39,7 +39,7 @@ function createServer(options) {
   server.on("connection", function(client) {
     client.once('set_protocol', onHandshake);
     client.once('login_start', onLogin);
-    client.once('ping_start', onPing);
+    //client.once('ping_start', onPing);
     client.on('end', onEnd);

     var keepAlive = false;
NODE_DEBUG=mc-proto node examples/server/server.js

Testing the ping client:

> node-minecraft-protocol $ node
// connection timeout calls callback
> require('./').ping({host:'github.com', port:25565}, function(){console.log(arguments)})
undefined
[…wait a few minutes…]
> { '0': 
   { [Error: connect ETIMEDOUT 192.30.252.128:25565]
     code: 'ETIMEDOUT',
     errno: 'ETIMEDOUT',
     syscall: 'connect',
     address: '192.30.252.128',
     port: 25565 } }

// protocol error calls callback
> require('./').ping({host:'github.com', port:80}, function(){console.log(arguments)})
undefined
> { '0': { [Error: Deserialization error for status.toClient : Read error for name : 84 is not in the mappings value] field: 'status.toClient' } }
{ '0': { [Error: read ECONNRESET] code: 'ECONNRESET', errno: 'ECONNRESET', syscall: 'read' } }

// using above example (commenting out ping_start), never responds (waited 10+ minutes)
> require('./').ping({host:'localhost', port:25565}, function(){console.log(arguments)})
undefined

deathcap avatar Jan 24 '16 21:01 deathcap

I guess the timeout time should be an option. What should be its default value ? (for example what value does the vanilla client uses ?)

rom1504 avatar Feb 02 '16 08:02 rom1504

just got burned by this today, I guess I need to write my own library. The only way to clean up such zombie pings is to restart the application. I quickly ran out of ports.

dawid2193487 avatar Apr 14 '19 13:04 dawid2193487

@ca1ek what about contributing instead of rewriting everything ?

rom1504 avatar Apr 14 '19 13:04 rom1504

In particular since that particular issue seems to be a few lines of code to fix.

rom1504 avatar Apr 14 '19 13:04 rom1504

I'd love to, but i'm not that good at JS. I thought about rewriting it in a language I'm more experienced with. Sorry if I sounded harsh

dawid2193487 avatar Apr 14 '19 14:04 dawid2193487

@rom1504 When you try to use the ip address of one of the Minecraft servers, endless execution occurs, response does not come:

IP: play.teslacraft.org

import ping from "minecraft-protocol/src/ping";

ping({ host: "play.teslacraft.org", port: 25565}, (error, data) => {
                    if (data) console.log(data);
                    if (error) console.log(error);
                });

I can assume that this is due to the fact that on this domain in addition to the server itself there is a site

Having caught the results, I came to the conclusion that with a net connection, this result is returned: https://github.com/PrismarineJS/node-minecraft-protocol/blob/8a143588a2bf2f0b0af4dca50dec2318206423bb/src/client/tcp_dns.js#L43

Socket {
  connecting: false,
  _hadError: false,
  _parent: null,
  _host: null,
  _readableState: ReadableState {
    objectMode: false,
    highWaterMark: 16384,
    buffer: BufferList { head: null, tail: null, length: 0 },
    length: 0,
    pipes: Splitter {
      _readableState: [ReadableState],
      readable: true,
      _events: [Object: null prototype],
      _eventsCount: 7,
      _maxListeners: undefined,
      _writableState: [WritableState],
      writable: true,
      allowHalfOpen: true,
      _transformState: [Object],
      buffer: <Buffer >,
      recognizeLegacyPing: true
    },
    pipesCount: 1,
    flowing: true,
    ended: false,
    endEmitted: false,
    reading: true,
    sync: false,
    needReadable: true,
    emittedReadable: false,
    readableListening: false,
    resumeScheduled: false,
    paused: false,
    emitClose: false,
    autoDestroy: false,
    destroyed: false,
    defaultEncoding: 'utf8',
    awaitDrain: 0,
    readingMore: false,
    decoder: null,
    encoding: null
  },
  readable: true,
  _events: [Object: null prototype] {
    end: [
      [Function: onReadableStreamEnd],
      [Function: endSocket],
      [Function]
    ],
    connect: [ [Function], [Function] ],
    error: [ [Function: onerror], [Function: onFatalError] ],
    close: [ [Function: endSocket], [Function] ],
    timeout: [Function: endSocket],
    data: [Function: ondata],
    unpipe: [Function: onunpipe],
    drain: [Function: pipeOnDrainFunctionResult],
    finish: [Function: bound onceWrapper] { listener: [Function: onfinish] }
  },
  _eventsCount: 9,
  _maxListeners: undefined,
  _writableState: WritableState {
    objectMode: false,
    highWaterMark: 16384,
    finalCalled: false,
    needDrain: false,
    ending: false,
    ended: false,
    finished: false,
    destroyed: false,
    decodeStrings: false,
    defaultEncoding: 'utf8',
    length: 0,
    writing: false,
    corked: 0,
    sync: true,
    bufferProcessing: false,
    onwrite: [Function: bound onwrite],
    writecb: null,
    writelen: 0,
    afterWriteTickInfo: null,
    bufferedRequest: null,
    lastBufferedRequest: null,
    pendingcb: 0,
    prefinished: false,
    errorEmitted: false,
    emitClose: false,
    autoDestroy: false,
    bufferedRequestCount: 0,
    corkedRequestsFree: {
      next: null,
      entry: null,
      finish: [Function: bound onCorkedFinish]
    }
  },
  writable: true,
  allowHalfOpen: false,
  _sockname: null,
  _pendingData: null,
  _pendingEncoding: '',
  server: null,
  _server: null,
  [Symbol(asyncId)]: 309,
  [Symbol(kHandle)]: TCP {
    reading: false,
    onconnection: null,
    [Symbol(owner)]: [Circular]
  },
  [Symbol(lastWriteQueueSize)]: 0,
  [Symbol(timeout)]: null,
  [Symbol(kBuffer)]: null,
  [Symbol(kBufferCb)]: null,
  [Symbol(kBufferGen)]: null,
  [Symbol(kBytesRead)]: 0,
  [Symbol(kBytesWritten)]: 0
}

As we see the host and port lines are null, so further connection does not work, I don’t know what can be done with this, but I think this information will be useful.

egorprnn avatar Mar 08 '20 16:03 egorprnn