fix(azure-iot-device-amqp): 🐛 multiple AMQP links were being created for telemetry #966
Conversation
|
/azp run node-canary |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| debug('using existing d2c link'); | ||
| this._d2cLink.send(amqpMessage, handleResult('AMQP Transport: Could not send', sendCallback)); | ||
| } else { | ||
| debug('waiting for a D2C link'); |
There was a problem hiding this comment.
Tell the story here in a comment.
| } | ||
| }); | ||
| } else { | ||
| if (this._d2cLink) { |
There was a problem hiding this comment.
You're going to have to add unit tests simply to keep our coverage numbers up AND, this should have some tests anyway.
|
/azp run node-canary |
|
Azure Pipelines successfully started running 1 pipeline(s). |
device/transport/amqp/src/amqp.ts
Outdated
| if (this._c2dLink) { | ||
| debug('C2D link already attached, doing nothing....'); | ||
| process.nextTick(callback); | ||
| } else if (this._c2dLinkAttaching) { |
There was a problem hiding this comment.
I think you face an issue that in the time betwwen starting to attach the c2d link and it's completion
- you get a new enable.
- you go ahead and on the next tick invoke the callback for the second enable which gives the implication that c2d is enabled when it's not.
- the actual enable then finishes 20 ticks later
Honestly, I think your solution to the send problem was pretty darn good. Can you use it here? If you can, you would then be for the most part be repeating code. Time to put enabling code into a function at that point? Darn booleans. Can't trust them.
There was a problem hiding this comment.
I see it. Good catch.
|
Looks good. We'll have to fix the gate though |
… explanation in comments
|
/azp run horton-node-gate |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run horton-node-gate |
|
Azure Pipelines successfully started running 1 pipeline(s). |
close #965
close #957