Skip to content
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

Include response payload in gRPC error responses #337

Open
bestbeforetoday opened this issue Nov 30, 2021 · 7 comments
Open

Include response payload in gRPC error responses #337

bestbeforetoday opened this issue Nov 30, 2021 · 7 comments
Labels
client Relates to Fabric Gateway client enhancement New feature or request server Relates to Gateway peer

Comments

@bestbeforetoday
Copy link
Member

As a blockchain application developer
I want my client application to receive payloads returned by chaincode error responses
So that I can pass metadata from the chaincode back to the client in the event of a failure

Currently only the error response message text is passed back as part of the gRPC status error message. The Gateway does associate ErrorDetails with this gRPC status error, so perhaps these details could be extended (or additional error metadata added) to include the payload.

It should be noted that using the error payload is not a very common pattern and is not supported by some of the older SDKs.

@bestbeforetoday bestbeforetoday added the enhancement New feature or request label Nov 30, 2021
@bestbeforetoday bestbeforetoday added this to the v1.1 milestone Feb 11, 2022
@bestbeforetoday bestbeforetoday removed this from the v1.1 milestone Jul 5, 2022
@bestbeforetoday bestbeforetoday added server Relates to Gateway peer client Relates to Fabric Gateway client labels Aug 1, 2023
@nalapon
Copy link

nalapon commented Feb 3, 2025

Within the fabric samples, asset-transfer-basic examples, we already have an example of how to handle this error.

i don't think that there is a reason for not being able to do this from the SDK.
My proposal is to put this error handling inside the SDK, so we can have a better message or even an error structure from which we can get more information.

I have a proposal, create an error struct, something like this:

{ “transaction_id”: “299.....”, “grpc_status”: “aborted”, “message”: “chaincode 500 response, asset70 asset does not exist”}

@bestbeforetoday
Copy link
Member Author

I'm not sure I completely understand your comment. The client API already provides a TransactionID field on the returned errors and, in the case of a CommitError, a Code field with the Fabric transaction validation code. The returned errors also satisfy the requirements of gRPC status package, and can be converted to gRPC Status structs, which in turn provide access to:

  • gRPC status code.
  • Error details.

The error details capture the response from each endorsing peer — there are typically more than one per transaction — and include:

  • Peer address.
  • MSP ID for the peer.
  • Error message returned from the chaincode transaction function.

As you can see from the samples, all the information you mention is available. It is just exposed using the gRPC status mechanism. I agree that it might be nice to make those error details more accessible / visible in the initial error object, as it is in the Node and Java implementations. Your proposal does not seem to achieve this since you have a single message field to cover multiple (potentially different) messages from endorsing peers.

I am open to alternative proposals. However, this needs to be captured in a different issue because this issue is not about peer error messages.

The transaction function response from the chaincode to the peer holds three pieces of information:

  1. Status code (this is a Fabric code, not a gRPC status code).
  2. Message (to hold a human-readable error message, which is what you are discussing).
  3. A binary payload (which is what this issue addresses).

In the case of a successful transaction endorsement, the binary payload is the return value from the transaction function. In the case of an error, the peer only returns the human-readable error message and discards the payload.

This issue suggests that the payload could be returned for the error case, in addition to the human-readable error message. The payload might be used for the smart contract transaction function to return some machine-readable information back to the calling application in the event of a rejected transaction invocation.

@nalapon
Copy link

nalapon commented Feb 4, 2025

Sorry because I think I have clearly not been very clear in the previous comment and I think I have also misunderstood the issue.

My previous comment refers to the fact that currently if you use the golang SDK, with the method SubmitTransaction

_, err := contract.SubmitTransaction("CreateAsset", assetId, "yellow", "5", "Tom", "1300")
   if err != nil {
	panic(fmt.Errorf("failed to submit transaction: %w", err))
}

and you print the error variable that you collect, it does not tell you the error, and you are forced to unpack the error from gRPC, which initially makes the development of applications quite confusing
My proposal was to try to create a clear error message out of the box, that in the error variable that you capture in SubmitTransaction already comes with the message that we have to unpack from the details in the gRPC structure.

As far as I see this issue was more along the lines, if I have not misunderstood once again, that the error should come with the payload of the transaction that you are sending. For example, if we send a transaction with the string “car”, that in the error comes the string “car” as payload. Am I wrong?

@bestbeforetoday
Copy link
Member Author

I think you are understanding correctly now.

I agree with you that it would be great to make the error details immediately available when printing the error. If you look at the example output and client code in the Running and Fabric Application tutorial, you see that the Node (TypeScript / JavaScript) implementation gives you the error details when writing the error to the console. The Java implementation is also enhanced to add the error details to the stacktrace of the Exception to produce similar results.

I did experiment with the Go implementation a while back to do something similar. The concern I had is whether this is idiomatic for Go. By convention you get returned a generic error interface, and need to use something like errors.As() to access anything other than the error message anyway. Go error messages are typically also a single line of text, perhaps because they are commonly composed (or wrapped) into higher-level error messages.

I would welcome suggestions you have on how best to implement for Go. Please raise a separate issue though, since this issue is specifically about including the response payload in addition to the response message(s).

@nalapon
Copy link

nalapon commented Feb 7, 2025

I've been tinkering around the code to see how to approach this issue and I have an idea. I hope is a good approach.

Currently in the /pkg/client/client.go we have the struct *gateway.EndorseRequest

func (client *gatewayClient) EndorseWithContext(
	ctx context.Context,
	in *gateway.EndorseRequest,
	opts ...grpc.CallOption,
) (*gateway.EndorseResponse, error) {

In the errors.go we can add the Payload to the TransactionError

// TransactionError represents an error invoking a transaction. This is a gRPC [status] error.
type TransactionError struct {
	*grpcError
	TransactionID string
	Payload       []byte // transaction proposal payload
}

If the transaction gives an error, you can unmarshal the SignedProposal and the common.Envelope struct until you reach the invocationSpec struct.

if err != nil {
		signedProposal := in.GetProposedTransaction()
		proposalBytes := signedProposal.GetProposalBytes()
		proposalStruct := new(peer.Proposal)

		.......

		args := invocationSpec.GetChaincodeSpec().GetInput().GetArgs()
		singleSliceArgs := bytes.Join(args, []byte(" "))

If I use this modification in the asset-transfer-basic example in the fabric-samples repo, gives the current error

*** Successfully caught the error:
Endorse error for transaction 56a7fb2e95a5dd48f4de9d9a3108bfb61be9c10245be4eb54439fc114d72b97b with gRPC status Aborted: rpc error: code = Aborted desc = 
failed to endorse transaction, see attached details for more info 
Payload: UpdateAsset asset70 blue 5 Tomoko 300
Error Details:
- address: peer0.org1.example.com:7051; mspId: Org1MSP; message: chaincode response 500, the asset asset70 does not exist

because endorseErr has the Payload attribute.

fmt.Printf(
    "Endorse error for transaction %s with gRPC status %v: %s Payload: %s\n",
    endorseErr.TransactionID,
    status.Code(endorseErr),
    endorseErr,
    endorseErr.Payload,
)

@bestbeforetoday
Copy link
Member Author

What you have extracted is the request (proposal) arguments sent to the transaction function. We already know this information because we sent it from the client application. There is no need to extract information from the proposal that we placed there in the first place.

This issue addresses the proposal Response payload, returned by the transaction function

@nalapon
Copy link

nalapon commented Feb 11, 2025

So my first assumption was wrong and the second was wrong too. Sorry that it’s taking me this long to understand the issue.

As far as i can remember the old SDK and the peer cli can retrieve the ProposalResponses and the peer.Response struct even when the endorse process fails.

I’ve been reading the fabric-protos and the gateway implementation, and correct me again if I’m wrong wich is very possible, to be able to retrieve the peer.Response as you told me, don’t we need to change the fabric-protos, the gateway server and then the client SDK?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Relates to Fabric Gateway client enhancement New feature or request server Relates to Gateway peer
Projects
None yet
Development

No branches or pull requests

2 participants