Skip to content

[DNM] hack: add Do-Not-Retry hint when retries are exhausted#2820

Draft
bjlaub wants to merge 8 commits into
developfrom
blaub/hack-exhausted-retries
Draft

[DNM] hack: add Do-Not-Retry hint when retries are exhausted#2820
bjlaub wants to merge 8 commits into
developfrom
blaub/hack-exhausted-retries

Conversation

@bjlaub
Copy link
Copy Markdown
Contributor

@bjlaub bjlaub commented Dec 16, 2025

Before this PR

After this PR

==COMMIT_MSG==
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link
Copy Markdown

changelog-app Bot commented Dec 16, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

[DNM] hack: add Do-Not-Retry hint when retries are exhausted

Check the box to generate changelog(s)

  • Generate changelog entry

import com.palantir.dialogue.futures.DialogueFutures;
import java.util.Optional;

// a channel that detects if upstream dialogue retries were exhausted, and fails fast if so
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.

oops, this comment is wrong - don't fail fast here, instead set a response attachment that causes RetryingChannel to short-circuit


private static final String DIALOGUE_RETRIES_EXHAUSTED_HEADER = "Dialogue-Retries-Exhausted";

public static boolean isRetriesExhausted(Response response) {
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.

we should separate the response attachment logic from the header parsing logic into two separate classes, the former being package-private

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant