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

UnhandledPromiseRejectionWarning #24

Open
mnemo70 opened this issue Aug 22, 2020 · 6 comments
Open

UnhandledPromiseRejectionWarning #24

mnemo70 opened this issue Aug 22, 2020 · 6 comments

Comments

@mnemo70
Copy link

mnemo70 commented Aug 22, 2020

Hi, thanks for your API module. I'm trying to write Node-RED nodes around it and on the first call to setProfile() it works, but a second call then raises this warning on the Node-RED console:

(node:15613) UnhandledPromiseRejectionWarning: #<ErrorEvent>
    at emitUnhandledRejectionWarning (internal/process/promises.js:151:15)
    at processPromiseRejections (internal/process/promises.js:211:11)
    at processTicksAndRejections (internal/process/task_queues.js:98:32)
(node:15613) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:15613) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
    at emitDeprecationWarning (internal/process/promises.js:161:11)
    at processPromiseRejections (internal/process/promises.js:213:13)
    at processTicksAndRejections (internal/process/task_queues.js:98:32)

I suspect that the calls to _request() in fetchMetrics() and setValues() trigger this warning or internal exceptions because I don't see any exception handling in there.

I'm using try/catch around the setProfile() call in my node, which should be okay. The catch does not get triggered.

if(newProfile >= 0) {
    (async () => {
        try {
            await client.setProfile(newProfile);
        }
        catch(e) {
            msg.payload = 'Exception setting profile to: ' + newProfile + ' Ex=' + e.message;
        }
    })()

Since I'm not an expert in Node.js, can you shed a light on this, please? Please excuse me if I'm totally wrong. :-)

Kind regards,
Thomas

@mnemo70
Copy link
Author

mnemo70 commented Aug 22, 2020

I just realized that this also happens in an example script. Using Node v14.8. On consecutive executions the the warning pops up sometimes, but sometimes not.

$ cat setProfile.js
const Vallox = require('@danielbayerlein/vallox-api');

(async () => {
  const client = new Vallox({ ip: '192.168.178.31', port: 80 })

  await client.setProfile(client.PROFILES.AWAY)
})()
$ node ./setProfile.js
(node:45214) UnhandledPromiseRejectionWarning: #<ErrorEvent>
(Use `node --trace-warnings ...` to show where the warning was created)
(node:45214) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:45214) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@danielbayerlein
Copy link
Owner

@mnemo70 Thank you for the report. I will solve this problem as soon as possible!

@danielbayerlein
Copy link
Owner

@mnemo70 Unfortunately I cannot reproduce the error. Can you please log the error using try...catch?

const Vallox = require('../');

(async () => {
  const client = new Vallox({ ip: '192.168.178.31', port: 80 })
  try {
    await client.setProfile(client.PROFILES.HOME)
  } catch (error) {
    console.log(error)
  }
})()

The IP address is correct?

@mnemo70
Copy link
Author

mnemo70 commented Aug 30, 2020

@danielbayerlein Yes, the IP is correct, of course. I have a Vallox ValloPlus 270 MV (type 3722) with software V2.0.2.

The warning does not trigger on every execution, maybe it's only reported when the connection fails in which case the Promise is rejected. What happens when you run into the rejection in your test?

I also have the problem that the API fails quite often in Node-RED with a "socket hang up" exception. But that would be another issue...

This is the result of your script:

$ node ./test.js
$ node ./test.js
$ node ./test.js
$ node ./test.js
(node:65672) UnhandledPromiseRejectionWarning: #<ErrorEvent>
(Use `node --trace-warnings ...` to show where the warning was created)
(node:65672) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:65672) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

$ node --trace-warnings ./test.js
(node:65673) UnhandledPromiseRejectionWarning: #<ErrorEvent>
    at emitUnhandledRejectionWarning (internal/process/promises.js:170:15)
    at processPromiseRejections (internal/process/promises.js:247:11)
    at processTicksAndRejections (internal/process/task_queues.js:94:32)
(node:65673) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:65673) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
    at emitDeprecationWarning (internal/process/promises.js:180:11)
    at processPromiseRejections (internal/process/promises.js:249:13)
    at processTicksAndRejections (internal/process/task_queues.js:94:32)
$

@mnemo70
Copy link
Author

mnemo70 commented Jan 20, 2021

What happens is that the WebSocket sometimes throws the Error "socket hang up". This results in the Promise from _request() being rejected in ws.onerror, but the calls to _request() do not handle the rejection.

For example, in fetchMetrics(), the call to _request() should be extended like this:

    const data = await this._request(buf.convertDataToBuffer(addy))
      .catch( (error) => {
        console.error('error in fetchMetrics: ' + error.message)
        // TODO handle the error
      })

It's the same for the call in setValues():

    await this._request(buf.convertDataToBuffer(VlxDevConstants.WS_WEB_UI_COMMAND_WRITE_DATA))
    .catch( (error) => {
        console.error('error in setValues: ' + error.message)
        return false
    })

This is the upper layer problem of correctly handling the Promise, but the underlying problem is with the WebSocket.

On my machine, it's always the call to setValues() in setProfile() that generates a "socket hang up" error. setProfile() first calls fetchMetrics() which hasn't thrown an error yet, then calls setValues(). I can imagine that the Vallox unit needs some time to handle the closing of the first WebSocket before it can process the seconds connection.

I'll analyze this further in the next days.

@mnemo70
Copy link
Author

mnemo70 commented Jan 20, 2021

I guess the gold standard would be to open the WebSocket connection only once and re-use it internally (or even allow it externally in the public API). This would relieve the Vallox unit from the costs of closing and re-opening the connection.

The easier way would be to just retry the operation a few times with a short delay.

I'll try to come up with some solution and submit a Pull Request.

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

No branches or pull requests

2 participants