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

Client.processResponse throws error without sending request #963

Closed
PierreSachot opened this issue May 23, 2024 · 3 comments
Closed

Client.processResponse throws error without sending request #963

PierreSachot opened this issue May 23, 2024 · 3 comments
Assignees
Labels

Comments

@PierreSachot
Copy link

PierreSachot commented May 23, 2024

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.
  • Library Version: 0.23.1
  • ROS Version: Humble
  • Platform / OS: Ubuntu 22.04.4 LTS

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 minggangw self-assigned this May 24, 2024
@minggangw
Copy link
Member

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

@minggangw
Copy link
Member

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
Copy link
Member

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

No branches or pull requests

2 participants