[kevinjonaitis-dot]

💸 TL;DR

A Thrift call can still report an earlier exception after its retry succeeds. Clear the response object before each attempt so the caller sees the latest response.

📜 Details

Thrift represents a method's response as a generated result struct containing either a success value or a declared service exception. The retry middleware reuses that same struct across attempts. Thrift decoding updates fields present in the response, but leaves fields absent from the response untouched.

For example, consider a method that returns a boolean:

1. The first attempt returns a retryable service exception. The decoder populates the result's exception field, and the middleware retries.
2. The next attempt succeeds and returns true. The decoder populates the success field. Since the successful response has no exception field, the previous exception remains in the reused struct.
3. The middleware checks the result for exceptions, finds the stale exception, and treats the successful attempt as another failure. If retries are exhausted, the caller receives an error despite the successful response.

This change zeroes the existing result struct before each attempt, keeping the caller's pointer intact. After a successful retry, the result contains the success value without the earlier exception. An exception returned by the final attempt is still retained. Retry policy and middleware order are unchanged.

Based on Ian Koeppe's fix in #724, with co-author credit. Baseplate owner: please confirm the required approval and merge process, and whether this fix should receive a version tag or release for downstream consumers.

🧪 Testing Steps / Validation

• Added unit tests.
• CI passed.

✅ Checks

• CI tests (if present) are passing
• Adheres to code style for repo
• Contributor License Agreement: not applicable (Reddit employee).

[konradreiche]

From the description alone, I don't understand the problem. Could we clarify this or give an example?

[fishy]

I believe the idea is that for example, say the thrift response struct is defined like:

struct Foo {
  1: optional TypeA field1
  2: optional TypeB field2
}

then the first attempt got field1 set with an exception, retry only has field2 set, but when writing the response on retry that sets field2 it would unset field1, so you get both field1 (from attempt 1) and field2 (from attempt 2) set in the end.

but in reality, thrift does not really permit you to set both response and exception, so when you get an retryable exception the response should always be nil, so this should not be a problem in reality. I'm not sure if there's some corner cases this could happen.

[kevinjonaitis-dot]

🤖/🧑‍🦱 @fishy, yes, you've got the failure sequence in your example. Each server response contains either success or an exception; the client ends up with both fields by reusing the same result object across attempts. Baseplate then extracts the old exception and reports the whole call as failed, even though the retry succeeded.

The reproduction steps are:

1. Configure two attempts and have the first RPC return a retryable declared exception. Thrift decodes it into the generated result object's exception field.
2. Baseplate retries the RPC, passing the same result object to the second attempt.
3. Have the second RPC return a successful response. Thrift populates the success field, but doesn't clear the exception field because that field is absent from the successful response.
4. The client-side result now contains success from attempt two and the exception from attempt one. Neither individual server response had both fields set.
5. The second call returns no error, but Baseplate's getClientError calls thrift.ExtractExceptionFromResult, finds the stale exception, and treats the successful attempt as another failure.
6. With both attempts used, Baseplate returns that error. The generated client returns early instead of returning the successful response to its caller.

We reproduced this with a generated client and the real Baseplate pool against a loopback Thrift server. Clearing the result before each attempt made the same exception-then-success sequence return the successful response.

cc @konradreiche