Skip to content

docs: plan injectable request reply callbacks - #56

Merged
bougyman merged 3 commits into
mainfrom
enable-injectable-request-reply-callbacks
Sep 15, 2026
Merged

bougyman merged 3 commits into
mainfrom
enable-injectable-request-reply-callbacks

Conversation

@bougyman

@bougyman bougyman commented Sep 14, 2026

Copy link
Copy Markdown
Member

Decision requested

Approve the proposed request/reply callback injection design before implementation begins.

Proposed design

  • Resolve request/reply callbacks from service configuration through a callable resolver.
  • Preserve the current NatsRequestReplyCallbacks behavior as the default resolver result.
  • Let applications supply an ordinary callback object that responds to #callbacks; it must not inherit from or mix in Leopard code.
  • Let application-composed policy decide logging and response behavior from the complete failure, including any status or other classification.

Non-goals

  • No runtime code, tests, or README behavior changes are in this PR.
  • This does not prescribe an application's failure type, status accessor, or status-policy mapping.

Acceptance criteria for follow-up implementation

  • Existing services retain current request/reply responses and error logging by default.
  • A service configures a resolver without monkey-patching or overriding Leopard internals.
  • The resolver receives logger: and returns a callback object with #callbacks.
  • A policy may choose a log level or no log entry for any failure status without changing respond_with_error(failure).
  • Exceptions retain their current behavior unless a supplied callback policy deliberately changes them.

Copilot AI lite review requested due to automatic review settings September 14, 2026 18:33
@bougyman
bougyman force-pushed the enable-injectable-request-reply-callbacks branch from 94902a5 to 66b3412 Compare September 14, 2026 18:36

This comment was marked as outdated.

@bougyman bougyman changed the title feat: inject request reply callbacks docs: plan injectable request reply callbacks Sep 14, 2026
@bougyman
bougyman force-pushed the enable-injectable-request-reply-callbacks branch from 66b3412 to 93e9b4e Compare September 14, 2026 18:39

@gabeodess gabeodess left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That should suite my needs!

Comment on lines +78 to +81
on_failure: ->(wrapper, result) do
@failure_log_policy.call(result.failure)
wrapper.respond_with_error(result.failure)
end,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would the update use @failure_log_policy as the default so we could inject that without overwriting callbacks?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

afaa5b2 Like sew?

Allow services to customize or suppress failure logging

NOTE: We're preserving the default error-level log and
      existing error response behavior, for now. We can
      make that injectable later.
Inline the default failure log policy and shorten the policy-injection test
to make YARD and RuboCop happy without changing callback behavior.
@bougyman
bougyman merged commit 21cc55b into main Sep 15, 2026
5 checks passed
@bougyman
bougyman deleted the enable-injectable-request-reply-callbacks branch September 15, 2026 19:12
bougyman pushed a commit that referenced this pull request Sep 16, 2026
This also adds the injectable error logging from #56, which should have been another feat: but I mistakenly pushed it as a docs:, so it didn't cut a release or hit the changelog.
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.

6 participants