Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
456 stars 169 forks source link

Error messages should contain an request object (like in await readHoldingRegisters(..)) #256

Closed vogsphar closed 7 months ago

vogsphar commented 4 years ago

On success (normal resp) i am not interested in the request, but when errors occur it would be good to know what was send to the Server.

alexbuczynsky commented 4 years ago

Can you give an example to describe what you are talking about?

vogsphar commented 4 years ago

A successfull response contains {metrics,request,response} The UserRequestError Object only contains {err,message,response} It would be helpfull to also add request (and metrics?) to UserRequestError {err,message,request,response}.

UserRequestError {
  err: 'ModbusException',
  message: 'A Modbus Exception Occurred - See Response Body',
  response: ModbusTCPResponse {
    _id: 2,
    _protocol: 0,
    _bodyLength: 3,
    _unitId: 24,
    _body: ExceptionResponseBody { _fc: 3, _code: 2 }
  }
}
{
  metrics: UserRequestMetrics {
    createdAt: 2020-02-21T07:56:33.867Z,
    startedAt: 2020-02-21T07:56:33.867Z,
    receivedAt: 2020-02-21T07:56:33.872Z
  },
  request: ModbusTCPRequest {
    _id: 1,
    _protocol: 0,
    _length: 6,
    _unitId: 24,
    _body: ReadHoldingRegistersRequestBody { _fc: 3, _start: 0, _count: 2 }
  },
  response: ModbusTCPResponse {
    _id: 1,
    _protocol: 0,
    _bodyLength: 7,
    _unitId: 24,
    _body: ReadHoldingRegistersResponseBody {
      _fc: 3,
      _byteCount: 4,
      _values: [Array],
      _bufferLength: 6,
      _valuesAsArray: [Array],
      _valuesAsBuffer: <Buffer 43 c8 01 48>
    }
  }
}
stefanpoeter commented 4 years ago

A successfull response contains {metrics,request,response}

Where is this happening?

alexbuczynsky commented 4 years ago

@stefanpoeter that is in v4.0

Here is an example: https://github.com/Cloud-Automation/node-modbus/blob/88bf1c20c75740b10e9adea8e6dcdc70795604a7/examples/typescript/tcp/ReadHoldingRegisters.ts#L16-L21

stefanpoeter commented 4 years ago

Ah, I never noticed :-) Well @vogsphar is right, it totally makes sense that this is in the error response as well. I don't think it would corrupt the API. I'll put it on this weeks todo list.

alexbuczynsky commented 4 years ago

Sounds good. Adding it should be fine. I can tackle adding it to the 4.0 version.