[service-bus] Fix issue on drain where onDrain and onMessage might sequence incorrectly.
#12772
Conversation
…n what the symptoms in Azure#12711 it seems to fit. The basic problem would be - the user calls receiveMessages(5), for instance, and the timeout expires before they receive them all so we request a drain. _Before_ the drain response comes we receive some of the other messages (after all, we requested 5). Those messages are then, via setTimeout, scheduled to be sent to our onMessage handler. When the drain response arrives (prior to this fix) the drain would be processed immediately (unlike other messages the rhea-promise code does not call it with setTimeout), potentially bypassing any message events that are still waiting to be processed in the task queue. By using setTimeout() here we use the same technique that the messages are scheduled by, ensuring that our resolve() is processed last.
richardpark-msft
requested review from
chradek,
HarshaNalluru and
ramya-rao-a
as code owners
ghost
added
the
|
This pull request is protected by Check Enforcer. What is Check Enforcer?Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass. Why am I getting this message?You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged. What should I do now?If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows: What if I am onboarding a new service?Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment: |
|
/azp run js - servicebus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…e the promise before message callbacks have completed.
|
/azp run js - servicebus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run js - servicebus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run js - servicebus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes an issue where drain could potentially resolve a promise even though some messages are pending processing.
This can happen if the user calls receiveMessages(5), for instance, and the timeout expires before they receive
them all - when this happens we then 'drain' the excess credits by sending a request to Service Bus on the link. Now, let's say that before the drain response comes in we receive some of the other messages (after all, we requested 5). Those messages are then, via setTimeout(emit, 0), scheduled to be sent to our onMessage handler. They are not dispatched to us immediately.
Now, when the drain response arrives (prior to this fix) the drain is processed immediately (unlike other messages the rhea-promise code does not call it with setTimeout), potentially bypassing any onMessage callbacks that are still waiting to be processed in the task queue.
By using setTimeout() here we use the same technique that the messages are scheduled by, ensuring that the resolve() is processed after any of those pending callbacks.
Fixes #12711