Skip to content

configuration for retry #26#41

Open
tolgaerdonmez wants to merge 3 commits intopratikbaid3:masterfrom
tolgaerdonmez:master
Open

configuration for retry #26#41
tolgaerdonmez wants to merge 3 commits intopratikbaid3:masterfrom
tolgaerdonmez:master

Conversation

@tolgaerdonmez
Copy link
Copy Markdown

For #26

I've added a retry configuration, I can modify, if you'll say so. just have a look and let's merge this.

@dolbin-prime
Copy link
Copy Markdown

Any progress? 🤔

I think this should be applied. I cannot control retrying connection, as it gets done unconditionally! 😥

@tolgaerdonmez
Copy link
Copy Markdown
Author

It's been a long time, I'll take a look and see what hapenned.

@pratikbaid3
Copy link
Copy Markdown
Owner

@tolgaerdonmez @dolbin-prime sorry for the delay. I'll get this PR reviewed and merged this weekend.

);
});
if (config.infinite || (config.tryCount ?? 0) > 0) {
Future.delayed(Duration(seconds: 5), () {
Copy link
Copy Markdown

@mjohlke mjohlke Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to hint to the html spec where they list a retry event which is meant to control the reconnection time in milliseconds.

Adding a field int reconnectTime to RetryConfiguration and use this instead of the static 5s for the timer might be a good starting point. What I am not sure about is if these retry-events should set reconnectTime automatically or if this should be up to the programmer

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a field int reconnectTime to RetryConfiguration and use this instead of the static 5s for the timer might be a good starting point.

I agree with this 🙏

@yanmingLiu
Copy link
Copy Markdown

@tolgaerdonmez @dolbin-prime sorry for the delay. I'll get this PR reviewed and merged this weekend.
This pr is required

@nitishabraham20
Copy link
Copy Markdown

@pratikbaid3 any update on this PR? this is an essential feature which will be beneficial. Kindly try merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants