Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Return failure traces when running with amqp_trx_plugin #9872

Merged
merged 3 commits into from
Jan 7, 2021

Conversation

heifner
Copy link
Contributor

@heifner heifner commented Jan 4, 2021

Change Description

  • Return full transaction trace on failure instead of just the exception when running with amqp_trx_plugin

Change Type

Select ONE:

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Testing Changes

Select ANY that apply:

  • New Tests
  • Existing Tests
  • Test Framework
  • CI System
  • Other

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

  • When running with amqp_trx_plugin, transaciton_trace can be returned for partial executed transactions which includes the exception. It is up to the user to understand these traces as failure cases.

plugins/producer_plugin/producer_plugin.cpp Show resolved Hide resolved
@@ -491,7 +491,8 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
bool process_incoming_transaction_async(const transaction_metadata_ptr& trx,
bool persist_until_expired,
next_function<transaction_trace_ptr> next,
RetryLaterFunc retry_later)
RetryLaterFunc retry_later,
bool return_failure_trace = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems pretty fragile to me. I think you'd be better of making sure that all downstream consumers can handle a transaction trace with an exception appropriately, and never extract the exception here. Either that or somehow use types that enforce consistency between return_failure_trace and next.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now this will only be true for amqp_trx_plugin. However, a new send_transaction2 in the future should also work this way (at least optionally). The community has requested this type of behavior.

Copy link
Contributor

@swatanabe-b1 swatanabe-b1 left a comment

Choose a reason for hiding this comment

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

Looks like that fixed the test failures.

@heifner heifner merged commit 13009bc into develop-boxed Jan 7, 2021
@heifner heifner deleted the failure-trace-dev-boxed branch January 7, 2021 16:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants