-
Notifications
You must be signed in to change notification settings - Fork 110
Fix: writing during short connection failures #7093
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
Allow changing a flag with different delays:
* Switch it on slow.
* Switch it off fast.
Useful for errors - so they only show after a while but are resolved quickly.
Example:
const input = ref(false)
const { delayed } = useDelayedFlag(input)
...
input.value = true
// 5 seconds later delayed.value will be true
...
input.value = false
// 200 ms later delayed.value will be true
Signed-off-by: Max <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7093 +/- ##
==========================================
+ Coverage 51.83% 51.86% +0.02%
==========================================
Files 477 478 +1
Lines 40681 40705 +24
Branches 988 993 +5
==========================================
+ Hits 21089 21112 +23
- Misses 19487 19488 +1
Partials 105 105 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Delay the error message and allow editing for 5 seconds. Also speed up the recovery by triggering push after a successful sync. Signed-off-by: Max <[email protected]>
520ce59 to
1676004
Compare
| if (timeout) { | ||
| clearTimeout(timeout) | ||
| } | ||
| const delay = val ? 5000 : 200 |
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.
Why would we still wait 200ms when switching back? Can't we do it instantly?
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.
My idea was to reduce the risk of a flickering state. Basically if the error goes away for a brief moment and comes back right after.
But thinking about it again I cannot see how that would happen within 200 ms. The error will only go away after the push request - which may be triggered by a successful sync. So then the next sync will be longer than 200 ms away. So I guess we can save the 200 ms and just remove the flag immediately.
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.
All right, that sounds good the still to keep.
juliusknorr
left a comment
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.
Small question inline, otherwise very nice
|
/backport to stable31 |
|
/backport to stable30 |
Fixes: #6877
enh(sync): add useDelayedFlag
Allow changing a flag with different delays:
Useful for errors - so they only show after a while but are resolved quickly.
Example:
const input = ref(false)
const { delayed } = useDelayedFlag(input)
...
input.value = true
// 5 seconds later delayed.value will be true
...
input.value = false
// 200 ms later delayed.value will be true
fix(sync): allow writing during short connection failures
Delay the error message and allow editing for 5 seconds.
Also speed up the recovery by triggering push after a successful sync.