Skip to content
This repository has been archived by the owner on Jan 13, 2023. It is now read-only.

Include reason for BadApiResponse in PromoteTransactionCommand #311

Closed
lzpap opened this issue Feb 24, 2020 · 4 comments
Closed

Include reason for BadApiResponse in PromoteTransactionCommand #311

lzpap opened this issue Feb 24, 2020 · 4 comments

Comments

@lzpap
Copy link
Member

lzpap commented Feb 24, 2020

PromoteTransactionCommand calls CheckConsistencyCommand internally to determine if the tail transaction can be promoted or not.
When status in CC response is False, BadApiResponse is raised which should contain the info field from CC response as well. This is the reason returned by the node.

@todofixthis
Copy link
Contributor

The response payload should already be included in the context attribute of the exception:
https://github.com/iotaledger/iota.py/blob/master/iota/adapter/__init__.py#L484-L496

@lzpap
Copy link
Member Author

lzpap commented Feb 25, 2020

@todofixthis I'm talking about this exception:

cc_response = CheckConsistencyCommand(self.adapter)(tails=[transaction])
if cc_response['state'] is False:
raise BadApiResponse(
'Transaction {transaction} is not promotable. '
'You should reattach first.'.format(transaction=transaction)
)

The response from the node should be alright from the adapter's perspective, it it should contain status, info and duration fields.

Btw a question: how are exceptions raised with with_context should be handled in application code? What is the best practice to get also the context and not just the error message written to the output?

@todofixthis
Copy link
Contributor

todofixthis commented Feb 25, 2020

with_context is meant to be a way for developers to include other relevant information with the exception that is more lightweight than trawling through the stack frames.

Including the information in the exception message is often convenient, but sometimes not (e.g., the context for an exception might be a bundle, and that means a VERY long exception message!).

Attaching additional information to the exception requires a bit of extra effort on the application developer's part, but it opens up additional options for troubleshooting.

E.g.,:

try:
  pt_response = api.promote_transaction(...)
except BadApiResponse as e:
  # Production scenario:
  # Log exception details to database, log, DLQ, etc. for forensics.
  log_to_database(e.message, e.context, # etc.

  # Development scenario:
  # Display context information with the exception, to make it easier to debug the issue.
  pprint(e.context)
  raise

In particular, I used this technique on a previous project that logged exceptions (and their context values) to a database, which proved invaluable for diagnosing errors that occurred on production.

Additionally, for developers who are not familiar with PyOTA internals, this gives them a way to inspect/debug values that are relevant to the exception, in a way that doesn't require them to edit/debug 3rd-party code.

@lzpap
Copy link
Member Author

lzpap commented Feb 26, 2020

Thanks @todofixthis for the explanation, I'm gonna include it in for #310.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants