Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java client throwing DecoderException on client.end #1087

Open
CyanCode opened this issue Feb 25, 2023 · 15 comments
Open

Java client throwing DecoderException on client.end #1087

CyanCode opened this issue Feb 25, 2023 · 15 comments

Comments

@CyanCode
Copy link

[✅ ] The FAQ doesn't contain a resolution to my issue

Versions

minecraft-protocol: 1.41.0
server: vanilla
node: 18.4.0

Detailed description of a problem

When calling client.end(...) on a connecting 1.19.2 client the connection is lost with the following message: Internal Exception: io.netty.handler.codec.DecoderException: aa: Non [a-z0-9_.-] character in namespace of location: {"text":"no server with subdomain localhost found"}

javaw_9kPvO8A7om

"no server with subdomain localhost found" is the message I intended to respond to the connecting client.

Current code

     const address = await getLocalAddress();
      const mcServer = mc.createServer({
        'online-mode': false,
        host: address,
        port: MC_SERVER_PORT,
        keepAlive: true,
      });

      mcServer.on('error', (e) => {
        this.logger.error('Error occurred while listening for logins', e);
      });
      mcServer.on(
        'login',
        async (client: ServerClient) => {
            // lines removed for brevity
            client.end("no server with subdomain localhost found");
        }
      );
      mcServer.on('listening', () => {
        this.logger.info(
          `Listening for login attempts on ${address}:${MC_SERVER_PORT}`,
        );
        resolve();
      }); 

Expected behavior

String passed into client.end(...) is what appears to the connecting Java client instead of the exception

Additional context

@CyanCode CyanCode changed the title Java client receiving DecoderException on client.end Java client throwing DecoderException on client.end Feb 25, 2023
@frej4189
Copy link
Contributor

Send the client a disconnect packet and then close the connection after instead

@CyanCode
Copy link
Author

Send the client a disconnect packet and then close the connection after instead

How do I send a disconnect packet (assuming this is a suggested change on my end and not one to the library). Will this apply to all previous versions as well or just 1.19+? Previously the string sent to client.end would simply appear on the MC client without any additional changes

@frej4189
Copy link
Contributor

frej4189 commented Feb 25, 2023

client.write('kick_disconnect', { reason: 'Reason for kicking the client' })

@CyanCode
Copy link
Author

Thanks I'll give that a try. Will this apply to all previous versions as well? i.e. I don't have to have separate paths for older versions using client.end and 1.19+ using client.write('kick_disconnect', ...), all versions will accept the disconnect packet.

@frej4189
Copy link
Contributor

The disconnect packet is present on all versions supported by nmp to my knowledge

@CyanCode
Copy link
Author

Great, thanks. Do we want to keep this issue open for the purpose of keeping client.end's implementation consistent across versions (e.g. used for relaying the passed reason to the client)? If not then the docs should probably be updated to include client.write('kick_disconnect', { reason: 'Reason for kicking the client' }) as an example method of disconnecting with a reason.

@frej4189
Copy link
Contributor

In my mind this is not something NMP should handle. Docs say otherwise, though. Gonna need an opinion from @rom1504

@extremeheat
Copy link
Member

Instead of updating client.end, a client.disconnect function can be added instead if it doesn't already exist. This is handled internally already in cases where the client needs to be disconnected (eg invalid auth), so should just be a matter of exposing it. Bedrock-protocol has a similar API function to do this.

@CyanCode
Copy link
Author

After sending the kick_disconnect packet the Java client is still running into the same exception (albeit with a slightly different message)

Function I'm calling

async function disconnect(reason: string) {
    await client.write('kick_disconnect', { reason });
}

image

@extremeheat
Copy link
Member

There are 2 different disconnect packets depending on the state of the client. You might be sending the wrong one, try disconnect, and make sure to run close afterwards.

That said, the error here seems completely unrelated. It's triggered by the client handling. You are likely sending invalid data to the client before calling .end. Don't send anything else (not even login) and check that disconnect works. If it works, you need to fix the packets you're sending previously.

@CyanCode
Copy link
Author

When you say disconnect and close afterwards are you suggesting something like this?

async function disconnect(reason: string) {
    await client.write('disconnect', { reason });
    await client.end();
}

Because doing so leads to a different error message
image

I also tried calling .end() after sending the kick_disconnect packet with the same error as before:

async function disconnect(reason: string) {
    await client.write('kick_disconnect', { reason });
    await client.end();
}

I did discover something though-- looking at the Client object in the debugger I'm seeing the version read 1.19.3, but the MC client I'm using is 1.19.2, is this expected behavior? If not I can try reinstalling Minecraft and see if something odd is happening with my local installation.

image

You are likely sending invalid data to the client before calling .end. Don't send anything else (not even login) and check that disconnect works. If it works, you need to fix the packets you're sending previously.

Also, I'm assuming login packets are being sent before the login event is called? i.e. connect event is called, NMP performs the login, and login event is called. Unless waiting for the login event is sending invalid data, I'm not sending any custom data (I'm really only using NMP to grab the connecting MC client's server host). Unfortunately I specifically need the login event to have occurred because I use the serverHost property which isn't available in the connect event's client object.

@extremeheat
Copy link
Member

That's not what I mean. This is the server example, and notice it sends a login packet.

client.write('login', {

I mean remove that and all the code below, and disconnect the client to make sure it works. Also, 1.19.3 and 1.19.2 are not compatible. Some things are broken in the current release, so you can downgrade node-minecraft-protocol to see if it fixes anything, npm install [email protected]

@CyanCode
Copy link
Author

I see-- while I am referencing the server example, I'm not sending any packets at all, basically all I'm doing is:

  1. Intercepting the login event
  2. Grabbing the host from client.serverHost and doing something unrelated to NMP with it
  3. Disconnecting with client.end(...) (up until this point where I've been trying client.write('kick_disconnect', { reason });)

The "Current code" section in the issue contains the only NMP code I'm running, the only interaction I'm having with the client is calling .end as soon as they login.

I think you're probably right that its just some things being broken in the current release. This very well may be fixed once those pending PRs are merged in.

@CyanCode
Copy link
Author

Seems like this is still an issue in the latest release-- is anyone else experiencing this when logging in with v1.19.2+ ?

@ImAhmadAbdullah
Copy link

I'm also getting a decoder exception, but a different error at the end: #1279

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants