RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
311 stars 70 forks source link

Client.processResponse throws error without sending request #963

Closed PierreSachot closed 5 days ago

PierreSachot commented 1 month ago

Description Rclnodejs execute function is triggering the client processResponse without having sent a request. This generates a new Error which is thrown and makes my server crash.

In order to debug it, we added the following logs inside rclnodejs: lib/client.js

/**
   * Send the request and will be notified asynchronously if receiving the repsonse.
   * @param {object} request - The request to be submitted.
   * @param {ResponseCallback} callback - Thc callback function for receiving the server response.
   * @return {undefined}
   * @see {@link ResponseCallback}
   */
  sendRequest(request, callback) {
    let requestToSend =
      request instanceof this._typeClass.Request
        ? request
        : new this._typeClass.Request(request);
    if (typeof callback !== 'function') {
      throw new TypeError('Invalid argument');
    }

    let rawRequest = requestToSend.serialize();
    let sequenceNumber = rclnodejs.sendRequest(this._handle, rawRequest);
    console.log("RCL ----------- SENDING REQUEST:", this._serviceName, sequenceNumber);
    debug(`Client has sent a ${this._serviceName} request.`);
    this._sequenceNumberToCallbackMap.set(sequenceNumber, callback);
  }

  processResponse(sequenceNumber, response) {
    console.log("RCL ----------- PROCESSING RESPONSE:", this._serviceName, sequenceNumber);
    if (this._sequenceNumberToCallbackMap.has(sequenceNumber)) {
      debug(`Client has received ${this._serviceName} response from service.`);
      let callback = this._sequenceNumberToCallbackMap.get(sequenceNumber);
      this._sequenceNumberToCallbackMap.delete(sequenceNumber);
      callback(response.toPlainObject(this.typedArrayEnabled));
    } else {
      throw new Error(
        `Client has received an unexpected ${this._serviceName} with sequence number ${sequenceNumber}.`
      );
    }
  }

and here are the outputs:

RCL ----------- SENDING REQUEST: /alpha/list_parameters 1
RCL ----------- PROCESSING RESPONSE: /alpha/list_parameters 1
RCL ----------- SENDING REQUEST: /alpha/describe_parameters 1
RCL ----------- PROCESSING RESPONSE: /alpha/describe_parameters 1
RCL ----------- SENDING REQUEST: /alpha/get_parameters 1
RCL ----------- PROCESSING RESPONSE: /alpha/get_parameters 1

RCL ----------- SENDING REQUEST: /bravo/list_parameters 1
RCL ----------- PROCESSING RESPONSE: /bravo/list_parameters 1
RCL ----------- SENDING REQUEST: /bravo/describe_parameters 1
RCL ----------- PROCESSING RESPONSE: /bravo/describe_parameters 1
RCL ----------- SENDING REQUEST: /bravo/get_parameters 2
RCL ----------- PROCESSING RESPONSE: /bravo/get_parameters 2

RCL ----------- SENDING REQUEST: /charlie/list_parameters 1
RCL ----------- PROCESSING RESPONSE: /charlie/list_parameters 1
RCL ----------- SENDING REQUEST: /charlie/describe_parameters 1
RCL ----------- PROCESSING RESPONSE: /charlie/describe_parameters 1
RCL ----------- SENDING REQUEST: /charlie/get_parameters 1
RCL ----------- PROCESSING RESPONSE: /charlie/get_parameters 1

RCL ----------- SENDING REQUEST: /delta/list_parameters 1
RCL ----------- SENDING REQUEST: /delta/list_parameters 1
RCL ----------- PROCESSING RESPONSE: /delta/list_parameters 1
RCL ----------- SENDING REQUEST: /delta/describe_parameters 1
RCL ----------- PROCESSING RESPONSE: /delta/describe_parameters 1
RCL ----------- SENDING REQUEST: /delta/get_parameters 1
RCL ----------- PROCESSING RESPONSE: /delta/get_parameters 1

RCL ----------- SENDING REQUEST: /echo/list_parameters 1
RCL ----------- PROCESSING RESPONSE: /echo/list_parameters 1
RCL ----------- SENDING REQUEST: /echo/describe_parameters 1
RCL ----------- PROCESSING RESPONSE: /echo/describe_parameters 1
RCL ----------- SENDING REQUEST: /echo/get_parameters 1
RCL ----------- PROCESSING RESPONSE: /echo/get_parameters 1

RCL ----------- PROCESSING RESPONSE: /alpha/get_parameters 1
/home/pierre/hmi/node_modules/rclnodejs/lib/client.js:76
      throw new Error(
      ^

Error: Client has received an unexpected /alpha/get_parameters with sequence number 1.
    at Client.processResponse (/home/pierre/hmi/node_modules/rclnodejs/lib/client.js:76:13)
    at /home/pierre/hmi/node_modules/rclnodejs/lib/node.js:218:20
    at Node._runWithMessageType (/home/pierre/hmi/node_modules/rclnodejs/lib/node.js:1654:5)
    at Node.execute (/home/pierre/hmi/node_modules/rclnodejs/lib/node.js:210:12)
Segmentation fault (core dumped)
A server error has occurred.
node exited with 139 code.

Steps To Reproduce

Currently we do not have a simple way to reproduce this bug. We are using clients to connect to the /${node}/describe_parameters same for get_parameters, list_parameters and set_parameters in order to retrieve them on our HMI.

We did a ros2 topic echo /parameter_events but this final response is triggered without any parameter change.

Expected Behavior Rclnodejs should ignore the response if there is no associated request. Also, rclnodejs should implement a way to catch this kind of errors.

Actual Behavior Rclnodejs crashes and this error is propagated to the server.

minggangw commented 1 month ago

@PierreSachot Thanks for your feedback, will take a look later.

minggangw commented 1 month ago

Hi @PierreSachot I'm busy preparing the release for latest ROS2 Jazzy, I saw you already had a deep investigation to identify the root cause, please feel free to open a PR to fix it, I will also touch on it when I have some free time, thanks!

minggangw commented 5 days ago

Fixed on https://github.com/RobotWebTools/rclnodejs/releases/tag/0.27.1