Skip to content
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

Add auto retry on interval #439

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

EinarArnason
Copy link
Contributor

Retries the failing URL on 5 second interval

Copy link
Member

@aperezdc aperezdc left a comment

Choose a reason for hiding this comment

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

@EinarArnason Hi there, thanks for the the patch! I am definitely not against the feature, but I would rather not change the existing behaviour by default. Would you consider making the retry interval a configurable option?

"</style></head><body>\n"
"</style>\n"
"<script>\nfunction retry() { window.location.href = '%s' }\n"
"setTimeout(retry, 5000);\n</script></head><body>\n"
Copy link
Member

Choose a reason for hiding this comment

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

This make the “retry every five seconds” behaviour unconditional, and I can imagine there may be cases in which this is not desired. It would be better if we could have an option to configure (or disable) the retry interval.

Copy link
Contributor Author

@EinarArnason EinarArnason May 23, 2022

Choose a reason for hiding this comment

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

You're right, an option would be best. Should it be disabled by default?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, disabled by default would keep the current behaviour by default, to avoid breaking people's setups. Thanks for considering reworking the patch 😸

@limbo127
Copy link

hello, can you commit this patch ?
Nicolas

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.

3 participants