Prevent unfulfilled promises on sendCommand method - #22
Conversation
| var commandPromise = Promise.race([ | ||
| const commandPromise = Promise.race([ | ||
| new Promise((resolve, reject) => { | ||
| this._commandResolves[command.id] = (c) => { |
There was a problem hiding this comment.
To prevent a memory leak caused by an error in this function execution, you can design it in that way:
() => {
try {
// logic here
} catch (err) {
return reject(...);
} finally {
delete this._commandResolves[command.id];
}
}
There was a problem hiding this comment.
We actually need a race of 2 promises here, because the command must fail if there was no response before the timeout
There was a problem hiding this comment.
I didn't question that. I'm talking about the arrow function you pass on the first promise.
There was a problem hiding this comment.
The try/catch block is a good way to prevent unhandled errors. By the way, it might help us to handle exceptions better at BLiP Portal
| setTimeout(() => { | ||
| if (!this._commandResolves[command.id]) | ||
| return; | ||
| if (!this._commandResolves[command.id]) { |
There was a problem hiding this comment.
Why do you need to check it?
There was a problem hiding this comment.
I just added the bracket, but this if is used to guarantee the promise will only be reject for commands with timeout
There was a problem hiding this comment.
That if statement seems to be impossible to be true.
There was a problem hiding this comment.
But anyway, I don't think that returning a string can be a good thing, think that consumers are expecting a specific type, in the case a lime envelope, returning a different type can cause problems. Miss you, typescript :(
Prevent keeping unfulfilled promises on memory because of SendCommand method.