-
Notifications
You must be signed in to change notification settings - Fork 83
fix (event processor) [OASIS-5907]: stop promise tracks in-flight dispatcher requests #397
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
Conversation
* limitations under the License. | ||
*/ | ||
|
||
class RequestTracker { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: please include a doc comment to describe this class
try { | ||
// swallow, an error stopping this queue should prevent this from stopping | ||
return this.queue.stop() | ||
this.queue.stop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be awaiting this also? Does it matter if onRequestsComplete resolves before this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's redundant. The Promise
returned from queue.stop
is based on the request it triggers at that time, but all requests are also tracked by the RequestTracker
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Hi! Will this resolve #382? |
Hi @clementallen, Yes, we released v3.5.0 with this change and it should solve the issue. Thanks for the suggestion. If you could try it out and let us know how it's working for you, we'd appreciate it. |
Summary
Add tracking functionality to
EventProcessor
for every request sent by its dispatcher. This is implemented in aRequestTracker
class.RequestTracker
exposes a method (onRequestsComplete
) to get aPromise
that fulfills after the completion of requests in-flight at the time of the call.stop
is updated to return the promise fromRequestTracker
onRequestsComplete
.Test plan
Issues
https://optimizely.atlassian.net/browse/OASIS-5907