Skip to content

reverseproxy: add lb_retry_match condition on response status - #7569

Merged
francislavoie merged 2 commits into
caddyserver:masterfrom
seroperson:i6267-reverse-proxy-retry
Apr 21, 2026
Merged

francislavoie merged 2 commits into
caddyserver:masterfrom
seroperson:i6267-reverse-proxy-retry

Conversation

@seroperson

@seroperson seroperson commented Mar 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #6267

  • lb_retry_match now handles expression option which evaluates against the upstream response via {rp.status_code}, {rp.status_text}, and {rp.header.X-Header} placeholders in CEL expressions
  • Added new helper function to CEL expressions isTransportError()
  • Using both expression + old syntax (I mean just a plain old lb_retry_match options) isn't allowed
  • Fully backward-compatible

It wasn't possible to implement it like lb_retry_match `path('/foo*') && {rp.status_code} in [502, 503]` because of how lb_retry_match is handled, but you can use a oneliner like lb_retry_match expression `path('/foo*') && {rp.status_code} in [502, 503]`

Everything is covered with unit-tests, e2e tests and caddyfile tests.

⚠️ Initially I wanted to preserve both old syntax + new syntax to make everything AND'ed, but it wasn't possible because of how tough the logic is: some things rely on existence of lb_retry_match options, some things not, and sometimes it's unclear whether something would be retried or not.

For example, the previous logic:

  reverse_proxy upstream1 upstream2 {
      lb_retries 3
  }

  ┌───────────────────────┬───────────────┐
  │       Scenario        │   Behavior    │
  ├───────────────────────┼───────────────┤
  │ Dial error            │ retried       │
  ├───────────────────────┼───────────────┤
  │ Transport error, GET  │ retried       │
  ├───────────────────────┼───────────────┤
  │ Transport error, POST │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Transport error, PUT  │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Any response          │ proxied as-is │
  └───────────────────────┴───────────────┘
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          method PUT
      }
  }

  ┌───────────────────────┬───────────────┐
  │       Scenario        │   Behavior    │
  ├───────────────────────┼───────────────┤
  │ Dial error            │ retried       │
  ├───────────────────────┼───────────────┤
  │ Transport error, GET  │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Transport error, POST │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Transport error, PUT  │ retried       │
  ├───────────────────────┼───────────────┤
  │ Any response          │ proxied as-is │
  └───────────────────────┴───────────────┘

GET loses its' default transport error retry because RetryMatch is non-nil and only PUT is defined. Adding expression handling would result into even more unclear logic. That's why using expression with plain options isn't allowed.

Just to remind what transport error is (roughly):

  • Upstream accepted the connection then reset it
  • TLS handshake succeeded but upstream closed during request/response transfer
  • Upstream timed out while sending the response body
  • Upstream sent malformed HTTP response headers

Now examples of how the new expression can be used:

  • Retry 502/503 responses only
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `{rp.status_code} in [502, 503]`
      }
  }

  ┌──────────────────────────────┬───────────────┐
  │           Scenario           │   Behavior    │
  ├──────────────────────────────┼───────────────┤
  │ Dial error, any method       │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, any method  │ not retried   │
  ├──────────────────────────────┼───────────────┤
  │ 502/503 response, any method │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 200/500 response             │ proxied as-is │
  └──────────────────────────────┴───────────────┘
  • Retry on response header only
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `{rp.header.X-Upstream-Retry} == "true"`
      }
  }

  ┌──────────────────────────────────────┬───────────────┐
  │               Scenario               │   Behavior    │
  ├──────────────────────────────────────┼───────────────┤
  │ Dial error, any method               │ retried       │
  ├──────────────────────────────────────┼───────────────┤
  │ Transport error, any method          │ not retried   │
  ├──────────────────────────────────────┼───────────────┤
  │ Response with X-Upstream-Retry: true │ retried       │
  ├──────────────────────────────────────┼───────────────┤
  │ Response without header              │ proxied as-is │
  └──────────────────────────────────────┴───────────────┘
  • Retry 503 only for POST
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `method('POST') && {rp.status_code} == 503`
      }
  }

  ┌─────────────────────────────┬───────────────┐
  │          Scenario           │   Behavior    │
  ├─────────────────────────────┼───────────────┤
  │ Dial error, any method      │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ Transport error, any method │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 503 POST                    │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ 503 GET                     │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 502 POST                    │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 200 POST                    │ proxied as-is │
  └─────────────────────────────┴───────────────┘
  • Retry 5xx for /api* paths only
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `path('/api*') && {rp.status_code} >= 500`
      }
  }

  ┌─────────────────────────────┬───────────────┐
  │          Scenario           │   Behavior    │
  ├─────────────────────────────┼───────────────┤
  │ Dial error, any method      │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ Transport error, any method │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 500 GET /api/foo            │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ 502 POST /api/foo           │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ 500 GET /other              │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 200 GET /api/foo            │ proxied as-is │
  └─────────────────────────────┴───────────────┘
  • Retry all transport errors + 502/503 responses
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() || {rp.status_code} in [502, 503]`
      }
  }

  ┌──────────────────────────────┬───────────────┐
  │           Scenario           │   Behavior    │
  ├──────────────────────────────┼───────────────┤
  │ Dial error, any method       │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, any method  │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 502/503 response, any method │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 200/500 response             │ proxied as-is │
  └──────────────────────────────┴───────────────┘
  • Retry GET transport errors + 502/503 responses
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `(isTransportError() && method('GET')) || {rp.status_code} in [502, 503]`
      }
  }

  ┌──────────────────────────────┬───────────────┐
  │           Scenario           │   Behavior    │
  ├──────────────────────────────┼───────────────┤
  │ Dial error, any method       │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, GET         │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, POST        │ not retried   │
  ├──────────────────────────────┼───────────────┤
  │ 502/503 response, any method │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 200 response                 │ proxied as-is │
  └──────────────────────────────┴───────────────┘
  • Retry PUT transport errors to /api* + any 502
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `(isTransportError() && method('PUT') && path('/api*')) || {rp.status_code} == 502`
      }
  }

  ┌────────────────────────────────────┬───────────────┐
  │              Scenario              │   Behavior    │
  ├────────────────────────────────────┼───────────────┤
  │ Dial error, any method             │ retried       │
  ├────────────────────────────────────┼───────────────┤
  │ Transport error, PUT /api/foo      │ retried       │
  ├────────────────────────────────────┼───────────────┤
  │ Transport error, PUT /other        │ not retried   │
  ├────────────────────────────────────┼───────────────┤
  │ Transport error, GET /api/foo      │ not retried   │
  ├────────────────────────────────────┼───────────────┤
  │ 502 response, any method, any path │ retried       │
  ├────────────────────────────────────┼───────────────┤
  │ 200 response                       │ proxied as-is │
  └────────────────────────────────────┴───────────────┘
  • Retry POST on 503, GET on transport errors
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `method('POST') && {rp.status_code} == 503`
      }
      lb_retry_match {
          expression `isTransportError() && method('GET')`
      }
  }

  ┌────────────────────────┬───────────────────┐
  │        Scenario        │     Behavior      │
  ├────────────────────────┼───────────────────┤
  │ Dial error, any method │ retried           │
  ├────────────────────────┼───────────────────┤
  │ Transport error, GET   │ retried (block 2) │
  ├────────────────────────┼───────────────────┤
  │ Transport error, POST  │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 503 POST               │ retried (block 1) │
  ├────────────────────────┼───────────────────┤
  │ 503 GET                │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 502 POST               │ not retried       │
  └────────────────────────┴───────────────────┘
  • Retry idempotent transport errors + 502 for specific host
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() && (method('GET') || method('PUT') || method('DELETE'))`
      }
      lb_retry_match {
          expression `host('api.example.com') && {rp.status_code} == 502`
      }
  }

  ┌──────────────────────────┬───────────────────┐
  │         Scenario         │     Behavior      │
  ├──────────────────────────┼───────────────────┤
  │ Dial error, any method   │ retried           │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, GET     │ retried (block 1) │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, PUT     │ retried (block 1) │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, DELETE  │ retried (block 1) │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, POST    │ not retried       │
  ├──────────────────────────┼───────────────────┤
  │ 502 to api.example.com   │ retried (block 2) │
  ├──────────────────────────┼───────────────────┤
  │ 502 to other.example.com │ not retried       │
  ├──────────────────────────┼───────────────────┤
  │ 200 response             │ proxied as-is     │
  └──────────────────────────┴───────────────────┘
  • Retry transport errors for multiple methods + 5xx for /api*
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() && (method('GET') || method('PUT'))`
      }
      lb_retry_match {
          expression `path('/api*') && {rp.status_code} >= 500`
      }
  }

  ┌────────────────────────┬───────────────────┐
  │        Scenario        │     Behavior      │
  ├────────────────────────┼───────────────────┤
  │ Dial error, any method │ retried           │
  ├────────────────────────┼───────────────────┤
  │ Transport error, GET   │ retried (block 1) │
  ├────────────────────────┼───────────────────┤
  │ Transport error, PUT   │ retried (block 1) │
  ├────────────────────────┼───────────────────┤
  │ Transport error, POST  │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 500 GET /api/foo       │ retried (block 2) │
  ├────────────────────────┼───────────────────┤
  │ 502 POST /api/foo      │ retried (block 2) │
  ├────────────────────────┼───────────────────┤
  │ 500 GET /other         │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 200 response           │ proxied as-is     │
  └────────────────────────┴───────────────────┘
  • Retry on response header + all transport errors
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() || {rp.header.X-Upstream-Retry} == "true"`
      }
  }

  ┌─────────────────────────────────┬───────────────┐
  │            Scenario             │   Behavior    │
  ├─────────────────────────────────┼───────────────┤
  │ Dial error, any method          │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ Transport error, any method     │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ 200 with X-Upstream-Retry: true │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ 502 with X-Upstream-Retry: true │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ 200 without header              │ proxied as-is │
  └─────────────────────────────────┴───────────────┘

@CLAassistant

CLAassistant commented Mar 14, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@seroperson

seroperson commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor Author

I've revisited this PR to make it simpler. Shortly, the only thing left is retrying based on status, which I moved to the lb_retry_match block.

I've removed retrying based on conditions as it messes with default behavior too much and it makes everything too complicated and unclear. I still can implement them in this PR, but feels like you should point me on how they must be defined and handled.

Updated the PR's description, previous one is now hidden under "Initial version" spoiler.

@seroperson seroperson changed the title feat(reverse_proxy): lb_retry_on conditions feat(reverse_proxy): lb_retry_match now contains status field Mar 16, 2026
@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch from b8aaa26 to 4e31ada Compare March 16, 2026 14:50
@seroperson

Copy link
Copy Markdown
Contributor Author

@mohammed90 Hello! Sorry for ping, but would you mind reviewing this?

@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch from eeb231c to d106d8c Compare March 25, 2026 00:16
@francislavoie

Copy link
Copy Markdown
Member

I find this quite awkward. As documented, the config layer says lb_retry_match takes a request matcher https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#lb_retry_match. But with this, it's a weird mix of request and response matchers. I'm not sure I like that. I understand the motivation though.

Could we use response matchers https://caddyserver.com/docs/caddyfile/response-matchers directly instead of statusCodes? It would allow the status code matching to come "for free", and add header matching support. I don't know if header matching is that useful, but it might be if the backend explicitly responds with some X-Retry-Me: true header or something lol

This also needs Caddyfile adapt tests. See .caddyfiletest files, there should be coverage for various patterns to cover backwards compat and new config.

@francislavoie francislavoie added the feature ⚙️ New feature or request label Mar 25, 2026
@francislavoie francislavoie changed the title feat(reverse_proxy): lb_retry_match now contains status field reverseproxy: add lb_retry_match condition on response status Mar 25, 2026
@seroperson

seroperson commented Mar 25, 2026 •

Copy link
Copy Markdown
Contributor Author

@francislavoie yea, the parsing looks weird in such scenario.

The reason why I did it exactly like this is that I wanted to preserve the ability to write the spec like this:

lb_retry_match {
  method POST
  status 503
}

This way it looks understandable for an end user and reads exactly like "retry on match method POST and status 503". We can preserve this approach and just re-use some parsing from response-matcher, so it will look like:

lb_retry_match {
  <request-matcher>
  <response-matcher>
}
// or like this
lb_retry_match {
  <request-matcher>
  on_response {
    <response-matcher>
  }
}

It will be still the same mix, but with re-used parsing logic.


OR we can split the matchers, so it will look like:

lb_retry_match {
  <request-matcher>
}
lb_retry_match_response {
  <response-matcher>
}

This way the code is cleaner and there is a strict separation of matching requests and responses, but this approach a bit less flexible because of how we treat conditions:

  • Currently every lb_retry_match is ORed and nested fields are ANDed, like lb_retry_match { ... AND ... } OR lb_retry_match { ... AND ... }.
  • With both lb_retry_match and lb_retry_match_response it intuitively feels like this: (lb_retry_match { ... AND ... } OR lb_retry_match { ... AND ... }) AND (lb_retry_match_response { ... AND ... } OR lb_retry_match_response { ... AND ... })

But then we will be unable to implement something like "retry on (DELETE and 401) or (POST and 503)". It will be only "retry on (DELETE or POST) and (401 or 503)".

Maybe I'm missing something.


@francislavoie @mohammed90 What do you think? Which approach to implement then?

@francislavoie

francislavoie commented Mar 27, 2026 •

Copy link
Copy Markdown
Member

Hmm.

I think we can just reframe this.

What we want is to match on response status or headers.

We can actually do this using CEL expressions already, as long as the Replacer is filled with properties of the response, placeholders can be used in the CEL expression to do complex conditions involving those.

We document in https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#intercepting-responses that {rp.status_code}, {rp.status_text}, and {rp.header.*} can be used in handle_response, we can ensure those are populated as well in the lb_retry codepath if it's not already. Then writing config like this should be possible:

lb_retry_match `path('/foo*') && {rp.status_code} in [502, 503]`

The backtick syntax might not work as-is in lb_retry_match, I don't remember how I implemented that, whether it only works on @named-matchers or if it would also work on subdirectives that take matchers too, but if not then the expression matcher name would need to be put in front of it.

We'll also need some .caddyfiletest Caddyfile -> JSON adapt fixtures for this functionality to prove is encodes properly.

@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch 2 times, most recently from 03fcc72 to 8b1f9c6 Compare March 28, 2026 15:26
@seroperson

seroperson commented Mar 28, 2026 •

Copy link
Copy Markdown
Contributor Author

@francislavoie thank you for the review and pointing in the right direction! I've revisited changes and re-implemented them.

Now:

  • lb_retry_match now handles expression option which evaluates against the upstream response via {rp.status_code}, {rp.status_text}, and {rp.header.X-Header} placeholders in CEL expressions
  • Added new helper function to CEL expressions isTransportError()
  • Using both expression + old syntax (I mean just a plain old lb_retry_match options) isn't allowed
  • Fully backward-compatible

It wasn't possible to implement it like lb_retry_match `path('/foo*') && {rp.status_code} in [502, 503]` because of how lb_retry_match is handled, but you can use a oneliner like lb_retry_match expression `path('/foo*') && {rp.status_code} in [502, 503]`

Everything is covered with unit-tests, e2e tests and caddyfile tests.

⚠️ Initially I wanted to preserve both old syntax + new syntax to make everything AND'ed, but it wasn't possible because of how tough the logic is: some things rely on existence of lb_retry_match options, some things not, and sometimes it's unclear whether something would be retried or not.

For example, the previous logic:

  reverse_proxy upstream1 upstream2 {
      lb_retries 3
  }

  ┌───────────────────────┬───────────────┐
  │       Scenario        │   Behavior    │
  ├───────────────────────┼───────────────┤
  │ Dial error            │ retried       │
  ├───────────────────────┼───────────────┤
  │ Transport error, GET  │ retried       │
  ├───────────────────────┼───────────────┤
  │ Transport error, POST │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Transport error, PUT  │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Any response          │ proxied as-is │
  └───────────────────────┴───────────────┘
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          method PUT
      }
  }

  ┌───────────────────────┬───────────────┐
  │       Scenario        │   Behavior    │
  ├───────────────────────┼───────────────┤
  │ Dial error            │ retried       │
  ├───────────────────────┼───────────────┤
  │ Transport error, GET  │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Transport error, POST │ not retried   │
  ├───────────────────────┼───────────────┤
  │ Transport error, PUT  │ retried       │
  ├───────────────────────┼───────────────┤
  │ Any response          │ proxied as-is │
  └───────────────────────┴───────────────┘

GET loses its' default transport error retry because RetryMatch is non-nil and only PUT is defined. Adding expression handling would result into even more unclear logic. That's why using expression with plain options isn't allowed.

Just to remind what transport error is (roughly):

  • Upstream accepted the connection then reset it
  • TLS handshake succeeded but upstream closed during request/response transfer
  • Upstream timed out while sending the response body
  • Upstream sent malformed HTTP response headers

Now examples of how the new expression can be used:

  • Retry 502/503 responses only
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `{rp.status_code} in [502, 503]`
      }
  }

  ┌──────────────────────────────┬───────────────┐
  │           Scenario           │   Behavior    │
  ├──────────────────────────────┼───────────────┤
  │ Dial error, any method       │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, any method  │ not retried   │
  ├──────────────────────────────┼───────────────┤
  │ 502/503 response, any method │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 200/500 response             │ proxied as-is │
  └──────────────────────────────┴───────────────┘
  • Retry on response header only
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `{rp.header.X-Upstream-Retry} == "true"`
      }
  }

  ┌──────────────────────────────────────┬───────────────┐
  │               Scenario               │   Behavior    │
  ├──────────────────────────────────────┼───────────────┤
  │ Dial error, any method               │ retried       │
  ├──────────────────────────────────────┼───────────────┤
  │ Transport error, any method          │ not retried   │
  ├──────────────────────────────────────┼───────────────┤
  │ Response with X-Upstream-Retry: true │ retried       │
  ├──────────────────────────────────────┼───────────────┤
  │ Response without header              │ proxied as-is │
  └──────────────────────────────────────┴───────────────┘
  • Retry 503 only for POST
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `method('POST') && {rp.status_code} == 503`
      }
  }

  ┌─────────────────────────────┬───────────────┐
  │          Scenario           │   Behavior    │
  ├─────────────────────────────┼───────────────┤
  │ Dial error, any method      │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ Transport error, any method │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 503 POST                    │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ 503 GET                     │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 502 POST                    │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 200 POST                    │ proxied as-is │
  └─────────────────────────────┴───────────────┘
  • Retry 5xx for /api* paths only
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `path('/api*') && {rp.status_code} >= 500`
      }
  }

  ┌─────────────────────────────┬───────────────┐
  │          Scenario           │   Behavior    │
  ├─────────────────────────────┼───────────────┤
  │ Dial error, any method      │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ Transport error, any method │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 500 GET /api/foo            │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ 502 POST /api/foo           │ retried       │
  ├─────────────────────────────┼───────────────┤
  │ 500 GET /other              │ not retried   │
  ├─────────────────────────────┼───────────────┤
  │ 200 GET /api/foo            │ proxied as-is │
  └─────────────────────────────┴───────────────┘
  • Retry all transport errors + 502/503 responses
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() || {rp.status_code} in [502, 503]`
      }
  }

  ┌──────────────────────────────┬───────────────┐
  │           Scenario           │   Behavior    │
  ├──────────────────────────────┼───────────────┤
  │ Dial error, any method       │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, any method  │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 502/503 response, any method │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 200/500 response             │ proxied as-is │
  └──────────────────────────────┴───────────────┘
  • Retry GET transport errors + 502/503 responses
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `(isTransportError() && method('GET')) || {rp.status_code} in [502, 503]`
      }
  }

  ┌──────────────────────────────┬───────────────┐
  │           Scenario           │   Behavior    │
  ├──────────────────────────────┼───────────────┤
  │ Dial error, any method       │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, GET         │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ Transport error, POST        │ not retried   │
  ├──────────────────────────────┼───────────────┤
  │ 502/503 response, any method │ retried       │
  ├──────────────────────────────┼───────────────┤
  │ 200 response                 │ proxied as-is │
  └──────────────────────────────┴───────────────┘
  • Retry PUT transport errors to /api* + any 502
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `(isTransportError() && method('PUT') && path('/api*')) || {rp.status_code} == 502`
      }
  }

  ┌────────────────────────────────────┬───────────────┐
  │              Scenario              │   Behavior    │
  ├────────────────────────────────────┼───────────────┤
  │ Dial error, any method             │ retried       │
  ├────────────────────────────────────┼───────────────┤
  │ Transport error, PUT /api/foo      │ retried       │
  ├────────────────────────────────────┼───────────────┤
  │ Transport error, PUT /other        │ not retried   │
  ├────────────────────────────────────┼───────────────┤
  │ Transport error, GET /api/foo      │ not retried   │
  ├────────────────────────────────────┼───────────────┤
  │ 502 response, any method, any path │ retried       │
  ├────────────────────────────────────┼───────────────┤
  │ 200 response                       │ proxied as-is │
  └────────────────────────────────────┴───────────────┘
  • Retry POST on 503, GET on transport errors
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `method('POST') && {rp.status_code} == 503`
      }
      lb_retry_match {
          expression `isTransportError() && method('GET')`
      }
  }

  ┌────────────────────────┬───────────────────┐
  │        Scenario        │     Behavior      │
  ├────────────────────────┼───────────────────┤
  │ Dial error, any method │ retried           │
  ├────────────────────────┼───────────────────┤
  │ Transport error, GET   │ retried (block 2) │
  ├────────────────────────┼───────────────────┤
  │ Transport error, POST  │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 503 POST               │ retried (block 1) │
  ├────────────────────────┼───────────────────┤
  │ 503 GET                │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 502 POST               │ not retried       │
  └────────────────────────┴───────────────────┘
  • Retry idempotent transport errors + 502 for specific host
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() && (method('GET') || method('PUT') || method('DELETE'))`
      }
      lb_retry_match {
          expression `host('api.example.com') && {rp.status_code} == 502`
      }
  }

  ┌──────────────────────────┬───────────────────┐
  │         Scenario         │     Behavior      │
  ├──────────────────────────┼───────────────────┤
  │ Dial error, any method   │ retried           │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, GET     │ retried (block 1) │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, PUT     │ retried (block 1) │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, DELETE  │ retried (block 1) │
  ├──────────────────────────┼───────────────────┤
  │ Transport error, POST    │ not retried       │
  ├──────────────────────────┼───────────────────┤
  │ 502 to api.example.com   │ retried (block 2) │
  ├──────────────────────────┼───────────────────┤
  │ 502 to other.example.com │ not retried       │
  ├──────────────────────────┼───────────────────┤
  │ 200 response             │ proxied as-is     │
  └──────────────────────────┴───────────────────┘
  • Retry transport errors for multiple methods + 5xx for /api*
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() && (method('GET') || method('PUT'))`
      }
      lb_retry_match {
          expression `path('/api*') && {rp.status_code} >= 500`
      }
  }

  ┌────────────────────────┬───────────────────┐
  │        Scenario        │     Behavior      │
  ├────────────────────────┼───────────────────┤
  │ Dial error, any method │ retried           │
  ├────────────────────────┼───────────────────┤
  │ Transport error, GET   │ retried (block 1) │
  ├────────────────────────┼───────────────────┤
  │ Transport error, PUT   │ retried (block 1) │
  ├────────────────────────┼───────────────────┤
  │ Transport error, POST  │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 500 GET /api/foo       │ retried (block 2) │
  ├────────────────────────┼───────────────────┤
  │ 502 POST /api/foo      │ retried (block 2) │
  ├────────────────────────┼───────────────────┤
  │ 500 GET /other         │ not retried       │
  ├────────────────────────┼───────────────────┤
  │ 200 response           │ proxied as-is     │
  └────────────────────────┴───────────────────┘
  • Retry on response header + all transport errors
  reverse_proxy upstream1 upstream2 {
      lb_retries 3
      lb_retry_match {
          expression `isTransportError() || {rp.header.X-Upstream-Retry} == "true"`
      }
  }

  ┌─────────────────────────────────┬───────────────┐
  │            Scenario             │   Behavior    │
  ├─────────────────────────────────┼───────────────┤
  │ Dial error, any method          │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ Transport error, any method     │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ 200 with X-Upstream-Retry: true │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ 502 with X-Upstream-Retry: true │ retried       │
  ├─────────────────────────────────┼───────────────┤
  │ 200 without header              │ proxied as-is │
  └─────────────────────────────────┴───────────────┘

@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch from 8b1f9c6 to f4ec226 Compare March 28, 2026 15:38
@francislavoie

Copy link
Copy Markdown
Member

I'm confused. That looks like way too many changes for this. Why do we need "isTransportError" and such? That seems very over engineered. It's also changing how the placeholders are being set outside of the retries, I don't understand why. And why is it blocking other matchers to be alongside expression? That's very arbitrary, nothing else does that.

In my mind, the change should've just been "make sure placeholders are set during the retry flow, and add tests". That's it really. Possibly also "allow omitting expression token if the first input to lb_retry_match is a backtick-wrapped token".

@seroperson

seroperson commented Mar 28, 2026 •

Copy link
Copy Markdown
Contributor Author

@francislavoie thank you for a quick response

It's also changing how the placeholders are being set outside of the retries, I don't understand why.

It's now set earlier because the old place executes after the place where we decide to retry. Without the moving these variables would be unset in retry logic, which must go before HandleResponse, as HandleResponse is actually writes bytes to a downstream. It doesn't change the HandleResponse behaviour, just variables are set earlier.

Also I've added header_keys tracking to clear stale headers from previous attempts and prevent situations when, for example, upstream A returned header A, but upstream B didn't and read that previously set header A instead.

Why do we need "isTransportError" and such?

Everything else (path, query, protocol etc) already available in expression out-of-box, there is nothing new added besides isTransportError. But let's imagine we don't have isTransportError. When user writes something like this:

lb_retry_match { expression `{rp.status_code} in [502, 503]` }

Then in case of transport error, that's what happening in retry logic:

if lb.RetryMatch == nil && req.Method != "GET" {
// by default, don't retry requests if they aren't GET
return false
}
match, err := lb.RetryMatch.AnyMatchWithError(req)
if err != nil {
logger.Error("error matching request for retry", zap.Error(err))
return false
}
if !match {
return false
}

We don't get into the very first if because lb.RetryMatch != nil (lb_retry_match is set), and this match, err := lb.RetryMatch.AnyMatchWithError(req) evaluates into false because: rp.status_code is nil at the moment (got transport error, so no response from an upstream at all), so nil in [502,503] returns false.

It makes all transport error non-retriable if expression is set and contains rp.* checks (they always likely will be false). Which makes the whole transport error retrying logic even more unclear for user:

  • Default behavior: on transport error only GET is retriable, other methods are not
  • lb_retry_match { method PUT }: on transport error only PUT is retriable
  • lb_retry_match { method PUT; expression {rp.status_code} in [502, 503] } (or just only expression): on transport error nothing is retriable

I didn't found easy ways of fixing this + not break the existing logic + not code workarounds + let API remain simple and not confusing for user.

And why is it blocking other matchers to be alongside expression?

That's why I separated expression from existing retry logic:

  • This way everything remains backward compatible
  • Transport errors now have a transparent way of handling with isTransportError()
  • Less footguns for users with mixing both plain options and expression (which also allows to use method, path etc)

If we somehow deal with transport error handling, then I guess there is no point to restrict mixing. Actually if we stick to isTransportError, then there is already no strict necessity, just I thought that it will be more clear for users to define everything either using old or new approach, but not mixing them (because options-only approach has its' own way of handling transport errors).

"make sure placeholders are set during the retry flow, and add tests". That's it really.

I also thought that initially, but it's not that simple. Besides things described above, I've also added additional handling of failure counting for health checks (so upstream marked unhealthy only in case of dial/transport errors), and the actual response matcher handling in reverseProxy: the pre-existed retrying logic is done only in tryAgain where response isn't available.

I can revert the isTransportError() and the mixing restriction, but then transport error handling will be unpredictable for users. The rest is necessary for things to work, though maybe I'm missing something and some things may be simplified.

@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch from f4ec226 to 0283a77 Compare March 28, 2026 23:59
Comment thread modules/caddyhttp/celmatcher.go Resolved
@seroperson

Copy link
Copy Markdown
Contributor Author
  • Removed mixing restriction
  • isTransportError() replaced with {rp.is_transport_error}
  • Added backtick shorthand in ParseCaddyfileNestedMatcherSet
  • Added DeleteByPrefix to Replacer
  • Removed CanMatchResponse() marker if favor of *MatchExpression type assertion

And updated all the tests, comments and docs everywhere.

@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch from 8f170bb to 61c00e9 Compare March 29, 2026 11:35

@francislavoie francislavoie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much better! I think I'm happy where this is at now. We'll just need to spend a bit of effort on documentation for the website as well (if you'd like to help with that after merging) to make it clear how the different states work, especially because of the hidden magic regarding branching logic if there's an expression vs not.

@seroperson
seroperson force-pushed the i6267-reverse-proxy-retry branch from 61c00e9 to 6087447 Compare March 29, 2026 15:01
@francislavoie francislavoie added this to the v2.11.3 milestone Mar 29, 2026
@francislavoie
francislavoie merged commit aed1af5 into caddyserver:master Apr 21, 2026
27 checks passed
@github-actions github-actions Bot mentioned this pull request May 11, 2026
4 tasks
@excid3

excid3 commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor

I've been trying to use this and am having some issues. My setup is a Caddy load balancer with Caddy upstreams that reverse proxy to localhost Rails apps.

For context, I'm trying to use this so passive health checks to upstreams where Rails might be down can retry automatically. Upstream Caddy would potentially return a 502, 503, or 504 in that case (I believe).

I'm getting CEL request matcher expects return type of bool, not google.protobuf.Any when using:

lb_retry_match expression `{rp.is_transport_error}`

I discovered this because using the combined expression with status codes was giving a no such overload error.

{"level":"error","ts":1783036764.2413948,"logger":"http.handlers.reverse_proxy","msg":"error matching request for retry","error":"no such overload"}

Using:

lb_retry_match expression `{rp.is_transport_error} || {rp.status_code} in [502, 503, 504]`

Is this a bug?


My full reverse proxy config is the following for context:

reverse_proxy x y {
  lb_try_duration 5s
  fail_duration 30s
  max_fails 2
  unhealthy_status 502 503 504 # Is this necessary with the lb_retry_match?
  lb_retry_match expression `method('GET') && ({rp.is_transport_error} || {rp.status_code} in [502, 503, 504])`
}

A few other questions:

  • Is the GET check here necessary? I only want to retry safe requests, but wasn't sure if this overrides the default checks in Caddy.
  • Do my unhealthy_status and lb_retry_match conflict in some way? It seems having both causes all my upstreams to be marked unhealthy, even though only one is unhealthy. Maybe some internal state doesn't get cleared on the retry so the second upstream gets marked unhealthy too on accident?

social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
caddy 2.11.3

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.

- [ ] `resource` blocks have been checked for updates.
<details>
  <summary>release notes</summary>
  <pre>This release improves several aspects of Caddy with minor features, bug fixes, and security patches. Thank you to everyone and their bots who contributed to help make this release the best one yet!

**Security patches:**

- fastcgi: Carrying over a patch from FrankenPHP for a bug that could allow non-PHP files to be executed; collaborated on by @dunglas, @KC1zs4, and @chenjj.
- vars: A more thorough fix for GHSA-m2w3-8f23-hxxf, collaborated by @everping and @vnxme.
- admin: Array index normalization to prevent remote admin socket auth bypass, by @Amemoyoi and bot.
- admin: More rigorous path prefix matching to prevent remote admin socket auth bypass, by @Amemoyoi and bot.

We've also merged a couple PRs that fix upstream security bugs in other projects like quic-go and CertMagic. Thank you to @marten-seemann for maintaining quic-go so diligently!

## What's Changed
* caddyhttp: Sync placeholder expansion in `vars` and `vars_regexp` by @vnxme in caddyserver/caddy#7573
* caddytls: Avoid ACME fallback for implicit Tailscale *.ts.net policies by @steadytao in caddyserver/caddy#7577
* chore: Resolve recent CI failures by @mholt in caddyserver/caddy#7593
* caddytls: Consolidate empty APs more smartly by @mholt in caddyserver/caddy#7567
* rewrite: skip query rename when source key is absent by @steadytao in caddyserver/caddy#7599
* root: introduce down-propagating Helper.BlockState for other directives/plugins to use by @henderkes in caddyserver/caddy#7594
* http: make zstd checksum configurable by @ottenhoff in caddyserver/caddy#7586
* notify: Always send "READY=1" even after an error by @francislavoie in caddyserver/caddy#7597
* reverseproxy: Fix check for `header_up Host {upstream_hostport}` redundancy by @yubiuser in caddyserver/caddy#7564
* caddytls: Expand placeholders in dns_challenge override_domain tls parameter by @pberkel in caddyserver/caddy#7609
* tls: add system and combined CA pool modules by @HarshPatel5940 in caddyserver/caddy#7406
* vars: Don't expand placeholders in values by @vnxme in caddyserver/caddy#7629
* build(deps): bump go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp from 1.42.0 to 1.43.0 by @dependabot[bot] in caddyserver/caddy#7637
* build(deps): bump the all-updates group across 1 directory with 11 updates by @dependabot[bot] in caddyserver/caddy#7641
* reverseproxy: make stream copy buffer size configurable by @steadytao in caddyserver/caddy#7627
* vars: Add matcher placeholder handling tests by @steadytao in caddyserver/caddy#7640
* build(deps): bump github.com/go-jose/go-jose/v4 from 4.1.3 to 4.1.4 by @dependabot[bot] in caddyserver/caddy#7621
* logging: Add journald encoder wrapper by @steadytao in caddyserver/caddy#7623
* caddyfile: Improve import/global options UX for imports before global options by @steadytao in caddyserver/caddy#7642
* chore: replace `interface{}` with `any` for modernization by @tsinglua in caddyserver/caddy#7571
* chore: bump timberjack to v1.4.1 by @DeRuina in caddyserver/caddy#7618
* logging: Preserve ts for journald-wrapped JSON logs by @steadytao in caddyserver/caddy#7644
* fileserver: show symlink targets verbatim (#7476) by @maxtruxa in caddyserver/caddy#7579
* fix(caddyfile): {block} in snippet by @prettysunflower in caddyserver/caddy#7558
* caddyhttp: Document missing placeholders for escaped URI and prefixed query by @steffenbusch in caddyserver/caddy#7659
* chore: add `AGENTS.md` by @mohammed90 in caddyserver/caddy#7652
* build(deps): bump github.com/jackc/pgx/v5 from 5.8.0 to 5.9.0 by @dependabot[bot] in caddyserver/caddy#7655
* admin: Redact sensitive request headers in API logs by @steadytao in caddyserver/caddy#7578
* reverseproxy: add `lb_retry_match` condition on response status by @seroperson in caddyserver/caddy#7569
* caddyhttp: prefer port 443 in auto-HTTPS and add tests by @mholt in caddyserver/caddy#7666
* fix: Propagate ECH keys to the QUIC listener by @steadytao in caddyserver/caddy#7670
* chore: Use atomics where appropriate by @francislavoie in caddyserver/caddy#7648
* metrics: Implement pushing via OLTP by @dunglas in caddyserver/caddy#7664
* logging: Add regression coverage for rotated file mode by @steadytao in caddyserver/caddy#7620
* httpcaddyfile: Inherit global ACME issuer settings in tls shortcuts by @steadytao in caddyserver/caddy#7617
* build(deps): bump github.com/jackc/pgx/v5 from 5.9.0 to 5.9.2 by @dependabot[bot] in caddyserver/caddy#7668
* admin: require path segment boundary in remote access control by @Amemoyoi in caddyserver/caddy#7673
* reverseproxy: Add ability to clear dynamic upstreams cache during retries by @mholt in caddyserver/caddy#7662
* listeners: clean up stale Unix socket files on Windows by @mfrischknecht in caddyserver/caddy#7676
* admin: reject non-canonical config array indices by @Amemoyoi in caddyserver/caddy#7592
* caddytls: Expand ACME credentials by @tribut in caddyserver/caddy#7554
* caddyauth: set user placeholders before auth rejection by @cyphercodes in caddyserver/caddy#7685
* caddyauth: revert user placeholders on auth rejection by @steadytao in caddyserver/caddy#7688
* chore: Fix golangci-lint 2.12.1 findings by @steadytao in caddyserver/caddy#7690
* httpcaddyfile: accept duration strings for log sampling interval by @tomholford in caddyserver/caddy#7694
* tls: Add alpn to managed HTTPS records by @steadytao in caddyserver/caddy#7653
* caddytls: avoid duplicate automation for wildcard-covered hosts by @Rijul-A in caddyserver/caddy#7697
* docs: add documentation for fileExists and fileStat template functions by @steffenbusch in caddyserver/caddy#7700
* rewrite: escape file matcher paths before rewriting by @cyphercodes in caddyserver/caddy#7683
* metrics: Add nil check for metricsHandler in AdminMetrics.serveHTTP by @Br1an67 in caddyserver/caddy#7553

## New Contributors
* @steadytao made their first contribution in caddyserver/caddy#7577
* @henderkes made their first contribution in caddyserver/caddy#7594
* @yubiuser made their first contribution in caddyserver/caddy#7564
* @pberkel made their first contribution in caddyserver/caddy#7609
* @HarshPatel5940 made their first contribution in caddyserver/caddy#7406
* @tsinglua made their first contribution in caddyserver/caddy#7571
* @maxtruxa made their first contribution in caddyserver/caddy#7579
* @seroperson made their first contribution in caddyserver/caddy#7569
* @Amemoyoi made their first contribution in caddyserver/caddy#7673
* @mfrischknecht made their first contribution in caddyserver/caddy#7676
* @tribut made their first contribution in caddyserver/caddy#7554
* @cyphercodes made their first contribution in caddyserver/caddy#7685
* @tomholford made their first contribution in caddyserver/caddy#7694
* @Rijul-A made their first contribution in caddyserver/caddy#7697
* @Br1an67 made their first contribution in caddyserver/caddy#7553

**Full Changelog**: https://github.com/caddyserver/caddy/compare/v2.11.2...v2.11.3</pre>
  <p>View the full release notes at <a href="https://github.com/caddyserver/caddy/releases/tag/v2.11.3">https://github.com/caddyserver/caddy/releases/tag/v2.11.3</a>.</p>
</details>
<hr>



See merge request: Harmonybrew/homebrew-core!842
steadytao pushed a commit that referenced this pull request Oct 9, 2026
…data (#8168)

* reverseproxy: Only retry responses for expressions that use response data

Since #7569 every lb_retry_match set with an expression is evaluated against
upstream responses. An expression that only tests the request, such as
{http.request.method} == "POST", then matched every successful response:
the request was sent again to the next upstreams and the client got a 502
once retries ran out. Only treat expressions referencing {rp.*} placeholders
as response conditions.

* reverseproxy: Only count placeholders set from the response as response data

* reverseproxy: Read response placeholders from the checked expression

The expression matcher records the placeholder names its checked CEL
expression reads, so retry_match finds direct ph() calls and ignores
escaped placeholder text in string literals. A placeholder name only
known at evaluation time counts as response data.

* reverseproxy: Match response placeholder names exactly

Only the header placeholders are matched by prefix now, and both
upstream latency placeholders are listed. MatchExpression.Placeholders
returns a copy, so callers cannot change the provisioned matcher.
NickCao pushed a commit to NickCao/caddy that referenced this pull request Nov 4, 2026
…data (caddyserver#8168)

* reverseproxy: Only retry responses for expressions that use response data

Since caddyserver#7569 every lb_retry_match set with an expression is evaluated against
upstream responses. An expression that only tests the request, such as
{http.request.method} == "POST", then matched every successful response:
the request was sent again to the next upstreams and the client got a 502
once retries ran out. Only treat expressions referencing {rp.*} placeholders
as response conditions.

* reverseproxy: Only count placeholders set from the response as response data

* reverseproxy: Read response placeholders from the checked expression

The expression matcher records the placeholder names its checked CEL
expression reads, so retry_match finds direct ph() calls and ignores
escaped placeholder text in string literals. A placeholder name only
known at evaluation time counts as response data.

* reverseproxy: Match response placeholder names exactly

Only the header placeholders are matched by prefix now, and both
upstream latency placeholders are listed. MatchExpression.Placeholders
returns a copy, so callers cannot change the provisioned matcher.

(cherry picked from commit 7c74563)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature ⚙️ New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom conditions for retrying proxy requests

5 participants