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

fix(kopiaui): Add timeout in pollOnce https.request equal to poll interval #3055

Merged
merged 2 commits into from Feb 21, 2024

Conversation

riedel
Copy link
Contributor

@riedel riedel commented Jun 4, 2023

was meant to fix #2453 (which I cannot reproduce with the current server part)

@riedel
Copy link
Contributor Author

riedel commented Jun 4, 2023

While this PR certainly can make sense IMHO i am not sure if it fixes #2453 or something else fixed it already, because I tried to recreate the problem with the latest source code without success (reverting this PR on my local code base).

To trigger the problem I IMHO one needs to get the server into a hanging state, which I could not: it returned 500s and some other errors now. However, the errors returned seem to be non-consistent, when I try to trigger them. I onl get y

(node:24124) electron: Failed to load URL: https://127.0.0.1:52703/api/v1/objects/7ae6d461baed99571532608ced577d62?fname=config with error: ERR_HTTP2_PROTOCOL_ERROR 

followed by (now instead of the hanging socket, I suspected):

11:51:02.417 > error fetching status Internal Server Error 
11:51:04.347 > error fetching status Internal Server Error 
11:51:07.351 > error fetching status Internal Server Error

Still: I think the PR make sense, so I mark it as ready.

@riedel riedel marked this pull request as ready for review June 4, 2023 10:11
@riedel riedel marked this pull request as draft June 4, 2023 10:42
@riedel
Copy link
Contributor Author

riedel commented Jun 4, 2023

Managed to reproduce the bug and this is not the non-timeouting socket.

@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb455c6) 75.86% compared to head (4aa0875) 77.16%.
Report is 15 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3055      +/-   ##
==========================================
+ Coverage   75.86%   77.16%   +1.29%     
==========================================
  Files         470      470              
  Lines       37301    28446    -8855     
==========================================
- Hits        28299    21950    -6349     
+ Misses       7071     4569    -2502     
+ Partials     1931     1927       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jkowalski jkowalski changed the title Add timeout in pollOnce https.request equal to poll interval fix(kopiaui): Add timeout in pollOnce https.request equal to poll interval Feb 18, 2024
@jkowalski
Copy link
Contributor

i think this still makes sense to merge, even if it's not a complete fix.

@jkowalski
Copy link
Contributor

Along with #3666 this should fix the socket exhaustion issue.

@jkowalski jkowalski marked this pull request as ready for review February 21, 2024 05:22
@jkowalski jkowalski merged commit c6536df into kopia:master Feb 21, 2024
27 of 28 checks passed
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.

Kopia(UI): Fails to close TCP connections and eventually causes unrelated processes to fail
2 participants