feat: introduce response-body-handler interface

[Barneey96]

The fix verifies the RTU CRC during parsing and exposes a corrupted flag mirroring the existing ModbusRTURequest behaviour, so a misparse realigns the buffer instead of consuming it. It also gives each transport a real resynchronization step: Modbus/TCP skips a complete-but-unparsable frame whole using the exact length in its MBAP header and drops a single byte when the header itself is implausible, while Modbus/RTU drops a byte whenever the function code cannot begin any response body or the CRC fails. Recovery from a desynchronizing frame falls from 25 request timeouts to 1 on TCP and from 38 to 1 on RTU, and the same 256 stray-byte cases now all return the correct reading immediately, with the buffer-size limit retained as a backstop for the one case nothing can prove — a frame stalled behind a valid function code.

[stefanpoeter]

Thanks a lot, i will review this as soon as I can. This seems to be a of a patch character so can you bump the patch version in package.json from 5.0.0 to 5.0.1.

[Barneey96]

Thanks a lot, i will review this as soon as I can. This seems to be a of a patch character so can you bump the patch version in package.json from 5.0.0 to 5.0.1.

Thank you, sure please take your time. I'm testing in the meanwhile with multiple ModbusTCP systems - so far looks great, but I'm open for feedback!

[stefanpoeter]

Hey @Barneey96,

sorry for the delay but I got time to look at your code.

Let's start with the TCP recovery, which I don't think we need. TCP itself handles validation and sequencing of packets, so the only thing we would be working against is a wrongly implemented counterpart — that's nothing we want to support. Either one speaks the right protocol or not.

The second part is the RTU recovery, and here is the hard part. RTU does get flipped bits and such — that's what the CRC checksum is for — but your changes remove the error response entirely, then search for the right function code in the remaining bytes. That is too unreliable and can lead to even more issues, since it is not unlikely to find an FC 3 or 4 in there. Even adding the expected_slaveAddress boosts the probability but still doesn't make it reliable.

On top of that, the corrupted=true path means CRC mismatches are no longer surfaced as err: 'crcMismatch' — they silently become timeouts instead, which is a breaking change for anyone handling that error.

So in all cases these changes introduce more problems than the original issues they are trying to solve.

What is your use case here? Where are you experiencing this?

[Barneey96]

Hi @stefanpoeter ,

Thank you so much for the detailed review!

I'm actually only experiencing the TCP-part, but I wanted to do a thorough job, so I touched both TCP and RTU.
I have a Pylontech FH3X inverter which occasionally sends a packet with the wrong count in it, which therefore corrupts also my following reads.
I agree that manufacturers should be able to speak the protocol corectly, but sometimes one must find a workaround, otherwise an inverter cannot be used in a relaible way.

First I did the following in my code:

    private discardStaleResponseBuffer(reason: string): void {
        // no public API for this, so the internals of the response handler have to be accessed directly
        const responseHandler = (this.client as any)?._responseHandler;
        const buffer = responseHandler?._buffer;

        if (!Buffer.isBuffer(buffer)) {
            log.warn(
                `Client '${this.name}/${this.id}': cannot discard the response buffer after ${reason}, jsmodbus` +
                    " internals changed - a truncated frame will desync this client until it's reconnected",
                LogScopes.UserRequestErrors,
            );
            return;
        }

        if (buffer.length > 0) {
            log.warn(
                `Client '${this.name}/${this.id}': discarding ${buffer.length} stale byte(s) from the response` +
                    ` buffer after ${reason}`,
                LogScopes.UserRequestErrors,
            );
            responseHandler._buffer = Buffer.alloc(0);
        }
    }

This works, but I'm not particularly happy with it because it requires accessing the internal _responseHandler and its _buffer directly. That makes the solution dependent on jsmodbus internals and therefore rather fragile.

I think there are essentially two ways this could be handled:

• jsmodbus could detect and recover from a malformed/truncated response automatically. In this case, the library would discard the invalid data and restore the connection to a usable state before processing the next request.
• Alternatively, the public API could provide a way to recover from this situation. For example, a clearResponseBuffer() or similar method would allow the application to explicitly discard the buffered data once it has detected that the connection has become desynchronized.

Either way, I think there should be a way to recover from a malformed response without having to access private/protected internals. In my case, the important part is that after receiving a malformed packet, I need to be able to discard the remaining bytes in the buffer so that subsequent requests can be processed normally.

I'll drop the RTU changes completely. Please advise on the TCP changes.

[stefanpoeter]

I totally understand your situation here. Dealing with faulty software makes life hard but I still don't think we should handle those cases in the main code. But we have some options since i think you have a valid use case here:

1. Fork the code yourself, create a patch for your case and then upstream changes from time to time. I know, not perfect.
2. Extract an interface for the response handler. When setting up a client i am ok when we can inject or set an interface to handle responses or such differently if you got a faulty response as it is in your case. So instead of tweaking internals one could setup a custom fc handler (or override it as it is in your case). its then basically copy and paste for you with your special fc behaviour.

I would go with 2 but you would have to present an PR for that. My take would be to intantiate a modbus client and then do something like client.registerRequestHandler(fc, interface)

What do you say?

[stefanpoeter]

Actually the correct way here would, of course be to contact the manufacturer and issue a bug ticket there. but I get that in the meantime you need a workaround and I think option 2 is a solid here.