feat: add timeout with cancellation to requests #1280
Conversation
beeme1mr
changed the title
beeme1mr
left a comment
beeme1mr
left a comment
There was a problem hiding this comment.
Hey @leoromanovsky, looks good so far. I just left a few thoughts.
| provider = objectOrUndefined<Provider>(domainOrProvider); | ||
|
|
||
| // Check if providerOrOptions is actually options | ||
| if (providerOrOptions && !('metadata' in providerOrOptions)) { |
There was a problem hiding this comment.
I'd like to find a more reliable way to test if it's a provider.
| this._logger.error((err as Error)?.stack); | ||
| } | ||
|
|
||
| private async withProviderTimeoutAndCancellation( |
There was a problem hiding this comment.
Should the provider continue to attempt to start or should we completely abort? If we want to abort, the error should be provider fatal and we may want to explicitly call the provider shutdown method.
@toddbaert @lukas-reining any thoughts on this?
|
Hey @leoromanovsky - we discussed this in the community meeting today. I understand the motivation for this, but we've tried to avoid this in the past. The reason is because this is already something the provider can control themselves. Many providers provide options, for example in their constructors, that allow users to specify initialization timeouts. The provider can then simply throw (or otherwise abnormally terminate) to signal an error during their initialization phase, and the SDK will react accordingly by throwing where appropriate, and running error handlers (this is what flagd does, for example). On the other hand, some providers don't require initialization at all, and are ready as soon as they are instantiated, making this option irrelevant for them. For this reason, we'd like to avoid adding this to the spec or SDKs. I do think adding abort signals is fine though; that's a different thing than a timeout option, more equivalent to the cc @lukas-reining @jonathannorris @kinyoklion @thomaspoignant |
3 participants
This PR
Related Issues
Fixes #1234523
Notes
Follow-up Tasks
How to test