-
Notifications
You must be signed in to change notification settings - Fork 721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Debug output for transaction submit #3819
Conversation
19e9cb7
to
055f872
Compare
7c15cc5
to
195c118
Compare
Sample run:
|
Sample JSON output: |
A few things:
We should not render Lets start with the above and I'll have another look. |
20dffe2
to
a5a779f
Compare
ac79543
to
d78820e
Compare
2b4c72d
to
2ebf82e
Compare
I removed the |
68ab868
to
862cbd2
Compare
3d49a63
to
555f88b
Compare
3a0cc45
to
3634496
Compare
3634496
to
19c3526
Compare
19c3526
to
29aa164
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of changes.
@@ -24,14 +33,20 @@ module Cardano.Api.InMode ( | |||
fromConsensusApplyTxErr, | |||
) where | |||
|
|||
import Prelude | |||
|
|||
import Cardano.Api.Eras |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the move of all the Cardano.Api.x
modules?
ShelleyTxValidationError ShelleyBasedEraAllegra (Consensus.ApplyTxError predicateFailures) -> toJSON (fmap toJSON predicateFailures) | ||
ShelleyTxValidationError ShelleyBasedEraMary (Consensus.ApplyTxError predicateFailures) -> toJSON (fmap toJSON predicateFailures) | ||
ShelleyTxValidationError ShelleyBasedEraAlonzo (Consensus.ApplyTxError predicateFailures) -> toJSON (fmap toJSON predicateFailures) | ||
ShelleyTxValidationError ShelleyBasedEraBabbage (Consensus.ApplyTxError _predicateFailures) -> Aeson.Null -- TODO implement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-- TODO implement
We should deal with this now
import Data.Aeson (FromJSON (..), ToJSON (..), object, (.=)) | ||
import qualified Data.Aeson as Aeson | ||
import Cardano.Api.Script | ||
import Cardano.Ledger.Alonzo.Rules.Bbody (AlonzoBbodyPredFail) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the import formatting of other Cardano.Api.x
modules
@@ -1,74 +1,169 @@ | |||
{-# LANGUAGE BangPatterns #-} | |||
{-# LANGUAGE BlockArguments #-} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢
, "failures" .= do map toJSON fs :: [Value] | ||
] | ||
|
||
instance ToJSON ChainPredicateFailure where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for example this ToJSON
instance should not be necessary. Why? Because ToObject ChainPredicateFailure
exists and the ToObject
class has a ToJSON
constraint: https://github.com/input-output-hk/iohk-monitoring-framework/blob/066f7002aac5a0efc20e49643fea45454f226caa/iohk-monitoring/src/Cardano/BM/Data/Tracer.lhs#L217
Can you double check that the orphan instances you have added are needed?
let errorAsText = Text.pack $ show err | ||
forM_ mErrorDetailFp $ \errorDetailFp -> | ||
liftIO $ LBS.writeFile errorDetailFp (Aeson.encode err) | ||
left $ ShelleyTxCmdTxSubmitError errorAsText |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also default the stdout to JSON? Ask QA what they think about this.
@@ -0,0 +1,136 @@ | |||
#!/usr/bin/env bash | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we want to add a new shell script just to see the failure.
45f2bfa
to
e2fd35b
Compare
e2fd35b
to
eb4a92c
Compare
New transaction submit --error-detail-out option Fail plutus script
eb4a92c
to
f2ef588
Compare
scripts/plutus/example-txin-locking-plutus-script-fail.sh
to trigger the error message.--error-detail-out
optionToJSON
instances for this featureToJSON
instances