Feat: Add http/http keyword timeout option #2142

zenyr · 2022-09-30T13:18:55Z

Tick the checkbox if you understand [x]:

I have read and understand the pull request rules.

Description

Adds timeoutMs to monitor and use it
- I added 'ms' to make sure the value is NOT a second, unlike intervals
Adds timeout_ms to sql patch list
Fixes support for response timeout setting in http monitor #2140, Fixes Add an option for timeout [Request Timeout] #2077, Fixes [Feature] Configurable Timeouts #877

Type of change

User interface (UI)
New feature (non-breaking change which adds functionality)
Translation update (minor and 🇰🇷 only, running script added a bunch of missing i18n data 😏 )
- edit: reverted non-english & non-Korean files

Checklist

My code follows the style guidelines of this project
I ran ESLint and other linters for modified files
I have performed a self-review of my own code and tested it
I have commented my code, particularly in hard-to-understand areas
(including JSDoc for methods)
My changes generate no new warnings

Screenshots (if any)

zenyr · 2022-09-30T13:23:04Z

src/pages/EditMonitor.vue

@@ -606,6 +611,10 @@ export default {
            if (this.monitor.retryInterval === oldValue) {
                this.monitor.retryInterval = value;
            }
+            // keep timeoutMs below interval, minding the unit conversion
+            if (!this.monitor.timeoutMs || this.monitor.timeoutMs >= value * 1000) {


Could it be be better to limit to 80% here? What are your thoughts? Your choice!

It might be a good idea to have some sort of limit to make sure that the previous request has definitely finished before the next one is sent

If you prefer less or equal than 80% of the interval seconds I can change it at once ;)
Please let me know. (cc. @louislam asking for owner's opinion)

Computroniks

Please don't add the English translations to non English files as it will fall back to the English translation if no other translation is found and will just cause merge conflicts later. I assume you ran npm run update-language-files.

@louislam Should we perhaps change this script so it doesn't update all the files and instead only changes a specific one? I don't really see any situation where it it useful and it only seems to create work for contributers when they run it and commit the results, only to be told to revert it.

zenyr · 2022-10-02T08:47:23Z

I see, let me update the PR to comply the best practice!

This reverts commit 349331a.

src/languages/ko-KR.js

zenyr · 2022-10-02T09:08:24Z

I forgot to commit the updated en.js, just pushed one! 😼

ps: We're using Uptime Kuma @ https://status.mymusictaste.net/

louislam · 2022-10-06T10:40:10Z

Please don't add the English translations to non English files as it will fall back to the English translation if no other translation is found and will just cause merge conflicts later. I assume you ran npm run update-language-files.

@louislam Should we perhaps change this script so it doesn't update all the files and instead only changes a specific one? I don't really see any situation where it it useful and it only seems to create work for contributers when they run it and commit the results, only to be told to revert it.

Oh yes, the command is bad.

zenyr · 2022-10-07T03:23:50Z

Please don't add the English translations to non English files as it will fall back to the English translation if no other translation is found and will just cause merge conflicts later. I assume you ran npm run update-language-files.
@louislam Should we perhaps change this script so it doesn't update all the files and instead only changes a specific one? I don't really see any situation where it it useful and it only seems to create work for contributers when they run it and commit the results, only to be told to revert it.

Oh yes, the command is bad.

Yeah, so I reverted all the artifacts that it created in this PR 😄 Since my team needs this feature and fixes a lot of pending issues is there something I could do to proceed with this PR? I'm sorry if I'm too attached to this feature but due to the fact our VPS network being flakey (like, losing a packet or few randomly) I really would like to use this feature in the stable version..! 😅

Thank you and ❤️ your work,
UK made me brave enough to hop into Vue train even though I've been a long time ⚛️ enthusiast.

Eliepse · 2022-11-25T08:35:37Z

Would love to see this feature merged. Is there anything that prevent it? I'm willing to help if I can :)

Eliepse · 2022-11-26T09:57:47Z

Just tested this PR, works fine for me 👍

usufu · 2023-01-05T03:23:56Z

@zenyr any docker image for this feature? or how to update docker image with this feature? I need this feature. thx.
why the pull request still not merged? @Computroniks @louislam

i-am-dev · 2023-01-26T11:50:07Z

Hi.

Any update on this PR and possibly when it will be merged?

Thanks.

i-am-dev · 2023-01-26T11:52:20Z

Hi.

Any update on this PR and possibly when it will be merged?

Thanks.

Nevermind. I see now that it is part of the 1.20.0 milestone.

Any due date for the release yet?

Thanks.

# Conflicts: # server/database.js # src/languages/en.js # src/languages/ko-KR.js

borgogelli · 2023-06-20T16:16:12Z

already a timeout notification

@lightingman117 you have written: "there is already a timeout notification"
But where is it ?

# Conflicts: # src/lang/ko-KR.json

louislam · 2023-07-08T13:26:39Z

The edit page is no longer working, here is the browser console log:

zenyr · 2023-07-14T07:34:04Z

src/pages/EditMonitor.vue

+            // limit to 80% of interval
+            const maxTimeout = this.monitor.interval * 0.8;
+            // 0 will be treated as 80% of interval
+            this.monitor.timeout = Math.Max(0, Math.min(timeout, maxTimeout));


This is a very amateur like mistake from my side. My bad!

zenyr · 2023-07-14T07:38:33Z

Just resolved "Math.Max" typo for the PR. Sorry for the hassle!
I must've been too excited when I added the line.

louislam · 2023-07-15T16:25:53Z

Test it again, unfortunately, newly created monitor doesn't seem to be working.

Weird default value:

I manually inputted 48 seconds, but the monitor timeout in 48ms. Forgot to x1000?

Old monitor, the original interval is 60s and the request time is 48s, then I change the interval to 30s.

The request time became this:

zenyr · 2023-07-16T04:58:59Z

Sure! I will ping you again once I finish testing the code first, I had issues setting up dev env on a new device(for personal reasons) but definitely will do!

zenyr · 2023-07-18T04:33:47Z

Fixed a few things, with proper dev setup this time 😅

Co-located timeout field with intervals, making their relation more clear
Used different formula to clamp to 80%, resulting better fraction numbers for edit/add view. ( t*0.8 => ~~(t * 8)/10) -- much unlikely to have 2.000000000004 epsilon error
Displays 80% inside the UI label that should directly hint 0 == default value(80%) (since timeout of 0 literally makes 0 sense)

This is not visible in the GIF below but in the source code, monitor.timeout || clampTimeout( interval ) is used.

Added db migration code to the last place, fixing current conflicts

src/pages/EditMonitor.vue

Co-authored-by: Frank Elsinga <frank@elsinga.de>

# Conflicts: # server/database.js # server/server.js # src/pages/EditMonitor.vue

zenyr added 2 commits September 30, 2022 21:51

feat: Add timeoutMs field

76ef467

chore: Update Languages (incl. ko-KR)

349331a

zenyr commented Sep 30, 2022

View reviewed changes

zenyr mentioned this pull request Sep 30, 2022

support for response timeout setting in http monitor #2140

Closed

1 task

Computroniks suggested changes Oct 1, 2022

View reviewed changes

zenyr added 2 commits October 2, 2022 17:47

Revert "chore: Update Languages (incl. ko-KR)"

d05324c

This reverts commit 349331a.

chore: Update ko-KR selectively

65fb26e

zenyr commented Oct 2, 2022

View reviewed changes

src/languages/ko-KR.js Outdated Show resolved Hide resolved

chore: Update en selectively

24749ce

zenyr requested a review from Computroniks October 2, 2022 09:13

Merge branch 'master' into feat-http-timeout

f4fb95d

This was referenced Nov 19, 2022

Can we sent notification on timeout? #2323

Closed

How long does Uptime Kuma wait for a response? Are we able to change it? #1922

Closed

update timeouts #889

Closed

louislam added this to the 1.20.0 milestone Nov 26, 2022

zenyr mentioned this pull request Nov 27, 2022

Up/down status based on the amount of ms #2337

Closed

1 task

chakflying mentioned this pull request Jan 18, 2023

What is the best recommendation for overcoming an ETIMEDOUT false alarm for a docker? #2630

Closed

2 tasks

louislam added 3 commits January 30, 2023 22:08

Merge remote-tracking branch 'origin/master' into feat-http-timeout

25d0977

# Conflicts: # server/database.js # src/languages/en.js # src/languages/ko-KR.js

Merge manually

794e1e6

Reorder and show only if http related monitors

0c74b5d

louislam added the question Further information is requested label Jan 30, 2023

CommanderStorm mentioned this pull request Jun 20, 2023

Error on long reponse time #3292

Closed

1 task

louislam removed the question Further information is requested label Jun 28, 2023

CommanderStorm mentioned this pull request Jul 5, 2023

Set ping ttl #3360

Closed

2 tasks

louislam added 2 commits July 8, 2023 21:12

Merge remote-tracking branch 'origin/master' into feat-http-timeout

bb8a2e2

# Conflicts: # src/lang/ko-KR.json

Put the sql patch at the end

c6d8b21

louislam added the question Further information is requested label Jul 8, 2023

louislam mentioned this pull request Jul 13, 2023

Down monitor notification is delayed and not immediately sent #3410

Open

2 tasks

Merge branch 'master' into feat-http-timeout

0f5c72b

zenyr commented Jul 14, 2023

View reviewed changes

Update EditMonitor.vue

47814f3

zenyr added 3 commits July 17, 2023 15:00

Merge remote-tracking branch 'upstream/master' into feat-http-timeout

a3a5e7e

Colocate timeout with retry, fix clampTimeout logic, show default on 0

9ace741

Merge remote-tracking branch 'upstream/master' into feat-http-timeout

d774855

chakflying mentioned this pull request Jul 18, 2023

增加响应时长和异常信息 #3440

Closed

1 task

CommanderStorm reviewed Jul 22, 2023

View reviewed changes

src/pages/EditMonitor.vue Outdated Show resolved Hide resolved

zenyr and others added 4 commits July 28, 2023 00:30

Update src/pages/EditMonitor.vue to remove a comment

e1ee816

Co-authored-by: Frank Elsinga <frank@elsinga.de>

Merge branch 'master' into feat-http-timeout

6d0bec3

# Conflicts: # server/database.js # server/server.js # src/pages/EditMonitor.vue

Fix merge issue

7d683be

Update the timeout value while finished editing the interval value

38bb4a4

louislam merged commit 439b651 into louislam:master Aug 6, 2023

SkwalExe mentioned this pull request Aug 8, 2023

Monitor shows google is down because of #2142 #3539

Closed

2 tasks

Nurgak mentioned this pull request Oct 8, 2023

Configurable Timeouts for MQTT #3855

Open

1 task

louislam mentioned this pull request Nov 9, 2023

Fix: Add axios abort signal #3961

Merged

7 tasks

chakflying mentioned this pull request Nov 11, 2023

Should be an ulitmate fix for request timeout issue #4011

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Add http/http keyword timeout option #2142

Feat: Add http/http keyword timeout option #2142

zenyr commented Sep 30, 2022 •

edited

Loading

zenyr Sep 30, 2022

Computroniks Oct 2, 2022

zenyr Oct 6, 2022

Computroniks left a comment

zenyr commented Oct 2, 2022

zenyr commented Oct 2, 2022 •

edited

Loading

louislam commented Oct 6, 2022

zenyr commented Oct 7, 2022

Eliepse commented Nov 25, 2022

Eliepse commented Nov 26, 2022

usufu commented Jan 5, 2023

i-am-dev commented Jan 26, 2023

i-am-dev commented Jan 26, 2023

borgogelli commented Jun 20, 2023

louislam commented Jul 8, 2023

zenyr Jul 14, 2023

zenyr commented Jul 14, 2023 •

edited

Loading

louislam commented Jul 15, 2023

zenyr commented Jul 16, 2023

zenyr commented Jul 18, 2023 •

edited

Loading

Feat: Add http/http keyword timeout option #2142

Feat: Add http/http keyword timeout option #2142

Conversation

zenyr commented Sep 30, 2022 • edited Loading

Description

Type of change

Checklist

Screenshots (if any)

zenyr Sep 30, 2022

Choose a reason for hiding this comment

Computroniks Oct 2, 2022

Choose a reason for hiding this comment

zenyr Oct 6, 2022

Choose a reason for hiding this comment

Computroniks left a comment

Choose a reason for hiding this comment

zenyr commented Oct 2, 2022

zenyr commented Oct 2, 2022 • edited Loading

louislam commented Oct 6, 2022

zenyr commented Oct 7, 2022

Eliepse commented Nov 25, 2022

Eliepse commented Nov 26, 2022

usufu commented Jan 5, 2023

i-am-dev commented Jan 26, 2023

i-am-dev commented Jan 26, 2023

borgogelli commented Jun 20, 2023

louislam commented Jul 8, 2023

zenyr Jul 14, 2023

Choose a reason for hiding this comment

zenyr commented Jul 14, 2023 • edited Loading

louislam commented Jul 15, 2023

zenyr commented Jul 16, 2023

zenyr commented Jul 18, 2023 • edited Loading

zenyr commented Sep 30, 2022 •

edited

Loading

zenyr commented Oct 2, 2022 •

edited

Loading

zenyr commented Jul 14, 2023 •

edited

Loading

zenyr commented Jul 18, 2023 •

edited

Loading