Skip to content

refactor: align timeout budget with wsman client for slow devices - #1153

Open
amarnath-ac wants to merge 1 commit into
mainfrom
1082_device_details_en_off_fix
Open

refactor: align timeout budget with wsman client for slow devices#1153
amarnath-ac wants to merge 1 commit into
mainfrom
1082_device_details_en_off_fix

Conversation

@amarnath-ac

@amarnath-ac amarnath-ac commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
  1. httpserver read/write timeout 15s to 40s so the wsman client (30s) times out first with a clean 504.
  2. waitForAuth 3s to 30s so concurrent handlers share one Target instead of forking, preserving the library concurrency cap.
  3. expireAfter 30s to 60s to keep the authenticated Target cached across rapid page refreshes.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.19%. Comparing base (7ccfa79) to head (78a6b6e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1153      +/-   ##
==========================================
+ Coverage   50.16%   50.19%   +0.02%     
==========================================
  Files         147      147              
  Lines       13574    13574              
==========================================
+ Hits         6810     6814       +4     
+ Misses       6172     6170       -2     
+ Partials      592      590       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@amarnath-ac
amarnath-ac force-pushed the 1082_device_details_en_off_fix branch from 30dcd5a to acbd654 Compare July 28, 2026 15:49
@amarnath-ac amarnath-ac changed the title fix: update to transport timeouts fix: align timeout budget with wsman client for slow devices Jul 28, 2026
@amarnath-ac
amarnath-ac marked this pull request as ready for review July 28, 2026 15:58
@amarnath-ac
amarnath-ac requested a review from a team as a code owner July 28, 2026 15:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts timeout and caching budgets so slow WSMAN operations are more likely to fail via the WSMAN client timeout (clean 504) rather than being cut off by the HTTP server or by duplicate concurrent authentication attempts.

Changes:

  • Increase default HTTP server timeouts to allow longer-running handlers.
  • Increase WSMAN connection-cache expiry to keep authenticated targets cached longer.
  • Increase the “wait for auth” window to reduce concurrent handlers forking multiple WSMAN clients for the same device.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg/httpserver/server.go Updates default HTTP server timeouts to a larger budget.
internal/usecase/devices/wsman/message.go Extends WSMAN connection cache TTL and auth-wait window to reduce duplicate client creation and keep targets warm longer.

Comment thread pkg/httpserver/server.go
Comment thread internal/usecase/devices/wsman/message.go Outdated
queueTickTime = 500 * time.Millisecond
expireAfter = 30 * time.Second // expire the stored connection after 30 seconds
waitForAuth = 3 * time.Second // wait for 3 seconds for the connection to authenticate, prevents multiple api calls trying to auth at the same time
expireAfter = 90 * time.Second // expire the stored connection after 90 seconds

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we make these configurable rather than hardcoded?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do it in a separate PR ? because these hardcoded values are spread across multiple places and need proper config wiring.

@sudhir-intc sudhir-intc changed the title fix: align timeout budget with wsman client for slow devices refactor: align timeout budget with wsman client for slow devices Aug 17, 2026
sudhir-intc
sudhir-intc previously approved these changes Aug 17, 2026
@amarnath-ac
amarnath-ac force-pushed the 1082_device_details_en_off_fix branch from faf6a06 to 0adadb2 Compare August 18, 2026 13:27
@amarnath-ac
amarnath-ac force-pushed the 1082_device_details_en_off_fix branch from 0adadb2 to 78a6b6e Compare August 18, 2026 14:03
1. httpserver read/write timeout 15s to 40s so the wsman client
  (30s) times out first with a clean 504.
2. waitForAuth 3s to 30s so concurrent handlers share one Target
   instead of forking, preserving the library concurrency cap.
3. expireAfter 30s to 60s to keep the authenticated Target cached
   across rapid page refreshes.
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.

4 participants