Add opcodes in list of operations

[msfeldstein]

It is difficult to find info on what horizon errors mean.

When a horizon call fails, it returns an opcode (tx_bad_auth for example) and sends you to our docs for more info. It says Descriptions of each code can be found at: https://www.stellar.org/developers/learn/concepts/list-of-operations.html" however when you get to that page if you try to search for your opcode nothing comes up. We should have the actual opcodes next to the horizon error codes.

tx_bad_auth probably is ALLOW_TRUST_TRUST_NOT_REQUIRED but i'm not really sure. It would be great to know for sure.

{
  "type": "https://stellar.org/horizon-errors/transaction_failed",
  "title": "Transaction Failed",
  "status": 400,
  "detail": "The transaction failed when submitted to the stellar network. The `extras.result_codes` field on this response contains further details.  Descriptions of each code can be found at: https://www.stellar.org/developers/learn/concepts/list-of-operations.html",
  "extras": {
    "envelope_xdr": "AAAAAFwDjFmQjkmPnH19otvP2uTV4jysFElqpOliy0QMET2RAAAAZAGECIwAAAADAAAAAQAAAAAAAAAAAAAAAF1wBy0AAAAAAAAAAQAAAAAAAAAGAAAAAVVTRFgAAAAAwgJ23a31Zxez/bwxmU6+6LOrn5BEE1gH0tUg2sanCd9//////////wAAAAAAAAABDBE9kQAAAEB8Ui9EHTSzkhpakuTuD6jYBI1UPUEN6+o4EY5muNhv0/xl7g7MTIj9mwKTelv7Wt653fiR/vjtuDRKVpR3YTAL",
    "result_codes": {
      "transaction": "tx_bad_auth"
    },
    "result_xdr": "AAAAAAAAAGT////6AAAAAA=="
  }
}

[msfeldstein]

Looks similar to #179 which was closed but I don't think it was solved

[bartekn]

For reference the codes can be found in codes package. I think that primary reason for this was to merge similar error codes into one, ex. all different operation success codes are now op_success. But you are right we should either update the docs or display original errors returned by stellar-core in Horizon response.

[ire-and-curses]

I don't really see the value in merging the similar codes into a smaller number of generic codes. Whether generic or not the caller probably needs to track the context of what was sent, to choose an appropriate specific follow-up action to the error (e.g. retry vs abort). Also the original stellar core errors reflect the underlying XDR, and I think abstracting this here doesn't aid comprehension.

tx_bad_auth is slightly different though. This is an error at the transaction level, because Horizon validated your TX and concluded it would never work (due to bad/missing signature and/or bad network passphase). Documentation for this (and other Horizon validation checks) should be clearly linked in the returned error. We do have a decent doc page here so I think the issue is making sure we link to it.

So:

1. We should fix the docs to link to the Horizon errors (probably at the top of list of operations)
2. Overall I think I'm in favour of changing the codes to be explicit. This would be a pretty major breaking change. @TomQ @tomerweller @rice2000 what are your thoughts?

Update: I hadn't internalised that tx_bad_auth is actually a protocol level error. Thanks for pointing that out @tomerweller.

[msfeldstein]

Agree with eric, and as a tiny point i'd mention on that page that bad auth is often caused by signing for the right network

…

On Thu, Sep 5, 2019 at 11:00 AM Eric Saunders ***@***.***> wrote: I don't really see the value in merging the similar codes into a smaller number of generic codes. Whether generic or not the caller probably needs to track the context of what was sent, to choose an appropriate specific follow-up action to the error (e.g. retry vs abort). Also the original stellar core errors reflect the underlying XDR, and I think abstracting this here doesn't aid comprehension. tx_bad_auth is slightly different though. This is an error at the transaction level, because Horizon validated your TX and concluded it would never work (due to bad/missing signature and/or bad network passphase). Documentation for this (and other Horizon validation checks) should be clearly linked in the returned error. We do have a decent doc page here <https://www.stellar.org/developers/horizon/reference/errors/transaction-failed.html> so I think the issue is making sure we link to it. So: 1. We should fix the docs to link to the Horizon errors (probably at the top of list of operations) 2. Overall I think I'm in favour of changing the codes to be explicit. This would be a pretty major breaking change. @TomQ <https://github.com/TomQ> @tomerweller <https://github.com/tomerweller> @rice2000 <https://github.com/rice2000> what are your thoughts? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#507>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABHK37K6XRJPYT7IWFD6UDQIFCKFANCNFSM4ITVKIDQ> .

[tomerweller]

I agree with @ire-and-curses. The return codes are well defined and documented in the transactions xdr definitions (txBAD_AUTH example). Abstracting over these is causing us unnecessary pain.

We can import these error comments to the horizon documentation and even use them to generate human readable error messages in the response from a failed transaction submission.

[msfeldstein]

Where does ACCOUNT_MERGE_HAS_SUB_ENTRIES get converted to op_has_sub_entries? I think its important that we actually have the actually opcode listed so people can google for it.

[msfeldstein]

Regarding it being a breaking change, if that's too much to handle we can add the real opcodes in addition.

I also have a bug filed in js-sdk to surface these errors via the sdk, and if we implement that we could change this behavior under the covers to not break anything.
stellar/js-stellar-sdk#413

[tomerweller]

Main offender: https://github.com/stellar/go/blob/8affeed3c57d334f7a5e47099007259b80fccb75/services/horizon/internal/codes/main.go#L38

[rice2000]

It looks like this situation has been improved by the API Reference section of the new docs here: https://developers.stellar.org/api/errors/result-codes/

Is that sufficient to solve this issue?

[ire-and-curses]

As you point out @rice2000, the new docs have the Horizon errors and the mapping to XDR opcodes. I think this is sufficient for the docs issue. I'll open an issue on Horizon to discuss whether to make the breaking change to move to explicit opcodes in a future version of the API.