Skip to content

Add timeouts to the contexts in TestLoadSessionHandle#6022

Merged
agocs merged 1 commit into
mainfrom
cagocs/fix_proxy_handlers_test_timeouts
Dec 30, 2025
Merged

Add timeouts to the contexts in TestLoadSessionHandle#6022
agocs merged 1 commit into
mainfrom
cagocs/fix_proxy_handlers_test_timeouts

Conversation

@agocs

@agocs agocs commented Dec 30, 2025

Copy link
Copy Markdown
Contributor

Summary

#6011 wants to bump google.golang.org/grpc from v1.77.0 to v1.78.0. [1] That is causing TestLoadSessionHandle in github.com/pomerium/pomerium/proxy to time out after 10 minutes. The tests create new proxies and call ServeHTTP on test requests. That execution gets all the way to GetDataBrokerRecord in querier.go, where it stalls out on res, err := q.Query(ctx, req, grpc.WaitForReady(true)).

What I think is happening is
grpc.WaitForReady(true) makes q.Query wait for the GRPC client to connect to the server. grpc-go v1.78.0 fixes a bug where the client would stay in the IDLE state while trying to connect to the GRPC server. What happens now is the client moves to the TRANSIENT_FAILURE state. The WaitForReady option sees TRANSIENT_FAILURE and waits until that failure fixes itself.

By setting a timeout in the context, we pass that context to q.Query eventually, the timeout fires, and GetDataBrokerRecord returns, which is what the test expected all along.

[1]: I bisected the go.mod file in that branch to figure out which dependency was causing the test to time out.

Related issues

#6011
grpc/grpc-go#8710

User Explanation

This fixes a bug that was causing a test to fail. The test was relying on a bug in the grpc-go client, which was fixed.

Checklist

  • reference any related issues
  • updated unit tests
  • add appropriate label (enhancement, bug, breaking, dependencies, ci)
  • ready for review

@agocs agocs requested a review from a team as a code owner December 30, 2025 20:42
@agocs agocs added the bug Something isn't working label Dec 30, 2025
@agocs agocs requested a review from calebdoxsey December 30, 2025 20:42
@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 20605497863

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.002%) to 52.894%

Files with Coverage Reduction New Missed Lines %
pkg/storage/postgres/registry.go 1 75.41%
pkg/storage/postgres/backend.go 4 76.58%
Totals Coverage Status
Change from base Build 20602446315: 0.002%
Covered Lines: 29493
Relevant Lines: 55759

💛 - Coveralls

@agocs agocs merged commit 37af9a0 into main Dec 30, 2025
15 checks passed
@agocs agocs deleted the cagocs/fix_proxy_handlers_test_timeouts branch December 30, 2025 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants