momenso / node-dht-sensor

Node.js Humidity and Temperature sensor addon
GNU Lesser General Public License v3.0
309 stars 72 forks source link

Non-resolving/rejecting promises #112

Closed JohnLindahlTech closed 4 years ago

JohnLindahlTech commented 4 years ago

To simulate a broken sensor for writing error-handling code I found out that invalid config does not resolve/reject promises.

Setup:

Actual Result:

It will never resolve nor reject.

Expected result:

After a while it should reject with some kind of timeout error.

Problem:

My current usecase is to read this "once in a while" (setInterval style). But I am not able to write error-handling for stuff like: invalid config (wrong pin) / broken sensors. So I can not identify the error. Sure, I can write my own timeout, but the underlying problem is still that it will allocate more and more promises which in turn hogs more resources.

JohnLindahlTech commented 4 years ago

In the end I ended up with using something like this, without the core-provided promise-implementation.

import sensor from 'node-dht-sensor';

const DEFAULT_TIMEOUT = 30 * 1000;

function read(type, pin, timeout = DEFAULT_TIMEOUT){
    return new Promise((resolve, reject) => {
        let timedOut = false;
        let done = false;
        const tId = setTimeout(() => {
            if(!done){
                timedOut = true;
                reject(new Error(`E3: [DHT] timed out after ${timeout}`));
            }
        }, timeout);
        sensor.read(type, pin, (err, temperature, humidity) =>{
            done = true;
            clearTimeout(tId);
            if(timedOut){
                return;
            };

            if(err){
                reject(err);
            } else {
                resolve({temperature, humidity});
            }
        })
    });
}

Feel free to close this issue, if you feel that this does not fit in the actual library. But personally I do believe that the provided implementation has a problem with leaking resources, at least this should be documented.