Skip to content

Conversation

jbro
Copy link
Contributor

@jbro jbro commented Apr 11, 2025

Currently if we extract the DialInfo from a Request Context in a custom Transport during an active health check, then the Upstream in the DialInfo will be nil.

This PR attempts to set the Upstream to a sensible value, based on wether or not the Upstream has been overridden in the active health check's config.

This is useful when using a custom Transport for the reverse_proxy module, as it makes GetDialInfo behave similarly in active health check requests and client requests.

Currently if we extract the DialInfo from a Request Context during an active health check, then the Upstream in the DialInfo is nil.

This PR attempts to set the Upstream to a sensible value, based on wether or not the Upstream has been overriden in the active health check's config.
@jesper-chainalysis jesper-chainalysis force-pushed the fix/add-upstream-to-dialinfo branch from 3a649d7 to c96f6d7 Compare April 14, 2025 06:20
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks! I think this makes sense, though it's been a while since I've been working with this code. We'll give it a shot and let you know if we have any issues with it :)

@mholt mholt merged commit 6c38ae7 into caddyserver:master Apr 15, 2025
20 checks passed
@mholt mholt added this to the v2.10.0-beta.5 milestone Apr 15, 2025
@jbro jbro deleted the fix/add-upstream-to-dialinfo branch May 12, 2025 19:05
mohammed90 pushed a commit to cedricziel/caddy that referenced this pull request Aug 29, 2025
…addyserver#6949)

Currently if we extract the DialInfo from a Request Context during an active health check, then the Upstream in the DialInfo is nil.

This PR attempts to set the Upstream to a sensible value, based on wether or not the Upstream has been overriden in the active health check's config.
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.

2 participants