chore(market)_: Increase market timouts to 20s #6386
+756
−98
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (112)
|
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
3 times, most recently
from
ccd3187 to
ffdf9a1
Compare
dlipicar
reviewed
Mar 5, 2025
circuitbreaker/circuit_breaker.go
Outdated
| StartTime time.Time | ||
| Duration time.Duration | ||
| Err error | ||
| IsTimeoutErr bool |
There was a problem hiding this comment.
hmm it looks weird to have this additional field when we've got the error itself, why not check the error for specific values wherever we need it?
There was a problem hiding this comment.
fixed
(initially I added this separately because it includes the hystrix error check). But on second thought, it's OK to make a function for this. Same for the duration
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
ffdf9a1 to
b8c35ce
Compare
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
b8e791b to
a6ac483
Compare
friofry
changed the title
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
155a23c to
fc5ec3e
Compare
saledjenic
approved these changes
Mar 6, 2025
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
5380b2a to
be94d81
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6386 +/- ##
===========================================
+ Coverage 60.93% 61.09% +0.15%
===========================================
Files 874 874
Lines 112386 112473 +87
===========================================
+ Hits 68488 68711 +223
+ Misses 35907 35766 -141
- Partials 7991 7996 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
f2bc689 to
b437a71
Compare
dlipicar
reviewed
Mar 6, 2025
| @@ -45,9 +79,14 @@ func (c *HTTPClient) DoGetRequest(ctx context.Context, url string, params netUrl | |||
| } | |||
|
|
|||
| var resp *http.Response | |||
| for i := 0; i < maxNumOfRequestRetries; i++ { | |||
| maxRetries := c.maxRetries | |||
| if maxRetries <= 0 { | |||
There was a problem hiding this comment.
I'd expect 0 retries to mean 0 retries, can we use just negatives to set the default value?
There was a problem hiding this comment.
or maybe rename number of attempts?
dlipicar
approved these changes
Mar 6, 2025
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
b437a71 to
e20db63
Compare
friofry
changed the title
* Can be tested with `FetchTokenDetails` on limited network (less than 1mbps) * updated /rpc/network in codeowners * chore(circuit-breaker)_: log request duration * chore(rpc-health-manager)_: add API method to get rpc stats * chore_: add total error count * chore_: fix double slash in http urls fixes status-im/status-mobile#22203
friofry
force-pushed
the
ab/issue-22203-increase-timeout
branch
from
e20db63 to
55e79f9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Main changes:
market.go- increased timeout for a market request (see details Zero balances in wallet status-mobile#22203 (comment))wallet/thirdparty/http_client.go- increase timeout from 5 seconds total to (5-5-5-60)Added more diagnostic information:
circuit_breaker.go- log additional responseSize (if any), duration, timeout cancellationFixes status-im/status-mobile#22203