New Error Handling Design

[volovyks]

Error Handling Issues and Proposal

What are the issues?

1. Not all the errors are typed

Nearcore JSON RPC Handlers (typed: [x] , untyped: [] ):

• broadcast_tx_async
• broadcast_tx_commit
• block
• chunk
• EXPERIMENTAL_broadcast_tx_sync
• EXPERIMENTAL_changes
• EXPERIMENTAL_changes_in_block
• EXPERIMENTAL_check_tx
• EXPERIMENTAL_genesis_config
• EXPERIMENTAL_light_client_proof
• EXPERIMENTAL_receipt
• EXPERIMENTAL_tx_status
• EXPERIMENTAL_validators_ordered
• gas_price
• health
• light_client_proof
• next_light_client_block
• network_info
• query
• status
• tx
• validators

Example (query -> view_account that does not exist):

{"jsonrpc":"2.0","error":{"code":-32000,"message":"Server error","data":"account nearkat456789088776567899.testnet does not exist while viewing"},"id":123}

Visible to developer as:

TypedError: [-32000] Server error: account nearkat456789088776567899.testnet does not exist while viewing
    at JsonRpcProvider.sendJsonRpc (/home/serhii/Projects/NEAR/near-api-js/lib/providers/json-rpc-provider.js:168:27)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async main (/home/serhii/Projects/NEAR/near_helper_scripts/simple_view_call.js:30:24) {
  type: 'UntypedError',
  context: undefined
}

It leads to code like this:

catch (error) {
	if (error.message.match(/Transaction nonce \d+ must be larger than nonce of the used access key \d+/)) {
		// handle error
        }
}

} catch (error) {
	if (error.toString().indexOf(`ccount ${this.accountId} does not exist while viewing`) !== -1) {
}

2. broadcast_tx_* errors are typed in a nested way

JSON RPC response example:

{"jsonrpc":"2.0","error":{"code":-32000,"message":"Server error","data":{"TxExecutionError":{"InvalidTxError":{"InvalidNonce":{"tx_nonce":2,"ak_nonce":28}}}}},"id":124

InvalidNonce [Error]: Transaction nonce 2 must be larger than nonce of the used access key 28
    at Object.parseRpcError (/home/serhii/Projects/NEAR/near-api-js/lib/utils/rpc_errors.js:42:19)
    at JsonRpcProvider.sendJsonRpc (/home/serhii/Projects/NEAR/near-api-js/lib/providers/json-rpc-provider.js:156:40)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async main (/home/serhii/Projects/NEAR/near_helper_scripts/send--tokens-deconstracted.js:93:24) {
  type: 'InvalidNonce',
  context: undefined,
  ak_nonce: 28,
  tx_nonce: 2,
  kind: { tx_nonce: 2, ak_nonce: 28 }

Issues of this approach:

• no type field on nearcore response (it's added in near-api-js)
• data field is hard to understand
• lot's of redundant classes and complexity on near-api-js side
• potential problem with several versions of near-api-js in one project

3. No single pattern for error handling

3.1 JSON RPC Errors can be thrown in several places:

• https://www.notion.so/nearprotocol/Error-Handling-Issues-and-Proposal-b502c7f809184c7cb8aca37235ee36d9#20c36f4eaa4d4709a28fa64f5a521b01
• https://www.notion.so/nearprotocol/Error-Handling-Issues-and-Proposal-b502c7f809184c7cb8aca37235ee36d9#2f87d9cb6b7d411dab108756a9aff36d
• https://www.notion.so/nearprotocol/Error-Handling-Issues-and-Proposal-b502c7f809184c7cb8aca37235ee36d9#7c85b6a1869f4b93b05ae76636a89c04
• https://www.notion.so/nearprotocol/Error-Handling-Issues-and-Proposal-b502c7f809184c7cb8aca37235ee36d9#5b8a5c225b85469f9963b71a1569164b

It leads to unexpected behavior. For example, when we are using sendJsonRpc method without wrapper methods, such as json-rpc-provider.query().

3.2 Not all the errors that thrown from near-api-js are TypedError's.

3.3 Errors can be reported from nearcore in several ways, which is confusing:

{
  "jsonrpc": "2.0",
  "error": {
    "code": -32000,
    "message": "Server error",
    "data": {
      "TxExecutionError": {
        "InvalidTxError": {
          "InvalidNonce": {
            "tx_nonce": 2,
            "ak_nonce": 28
          }
        }
      }
    }
  },
  "id": 124
}

{
    "id": "dontcare",
    "jsonrpc": "2.0",
    "result": {
        "block_hash": "dAaoSGEVLsNGGZgzbNWo5hoxhuP891dap2kgXc3v8T3",
        "block_height": 37490290,
        "error": "access key ed25519:H9k5eiU4aXS3M4z8HzKJSLaZdqGdGwBG49o7orhC4eZW does not exist while viewing",
        "logs": []
    }
}

HTTP/1.1 200 OK
Alt-Svc: clear
Via: 1.1 google
content-length: 150
content-type: application/json
date: Mon, 22 Feb 2021 14:12:51 GMT

{
    "error": {
        "code": -32000,
        "data": "account nearkat234567.testnet does not exist while viewing",
        "message": "Server error"
    },
    "id": "dontcare",
    "jsonrpc": "2.0"
}

Response.result:
{
  status: { Failure: { ActionError: [Object] } },
  transaction: {
    signer_id: 'volovyk.testnet',
    public_key: 'ed25519:DHtE8z9bSdP9LykTSNKrKdi875pYFRWn6wx2SBGHsDwj',
    nonce: 33,
    receiver_id: 'serhii2888.testnet',
    actions: [ [Object] ],
    signature: 'ed25519:Skb4DcyMuAprmWpBLTDTT6a1TMLXRyBisMRVdXspjA7iR3prbbbfekGxKmy73xyDzSzK5fRwmDZMFJHZXYKVUSu',
    hash: 'HktGV9nm8HZ6PQJjtLYAEt75BFkfJHuxkhAq9EfsnRWY'
  },
  transaction_outcome: {
    proof: [],
    block_hash: '61Ze7v8VZ6Leg7hL7azmiYqDCtkn41uP6J48rjA6FZZY',
    id: 'HktGV9nm8HZ6PQJjtLYAEt75BFkfJHuxkhAq9EfsnRWY',
    outcome: {
      logs: [],
      receipt_ids: [Array],
      gas_burnt: 223182562500,
      tokens_burnt: '22318256250000000000',
      executor_id: 'volovyk.testnet',
      status: [Object]
    }
  },
  receipts_outcome: [
    {
      proof: [],
      block_hash: '84Mtmmnqotqz3ABGpnt283mJrjTqwszqnmjJKzunXnyU',
      id: '264omZFzHrY1pFgAaYL4dLwqXzWiPjDSTpXWHr9TTki4',
      outcome: [Object]
    },
    {
      proof: [Array],
      block_hash: '8emnp5XmdDwvMEHLQ97wwM2eHc9pVvKLo9M6eZz6ZhhU',
      id: '2mxju9Zd1W8C7755BK2jRh2oeup8MEooEXpRvwmiqDZr',
      outcome: [Object]
    },
    {
      proof: [Array],
      block_hash: '8emnp5XmdDwvMEHLQ97wwM2eHc9pVvKLo9M6eZz6ZhhU',
      id: 'HjGUi1yj1KZCTtWfKiSTxQyiXTtDcd4rHZXKbnZUnHoV',
      outcome: [Object]
    }
  ]
}

Proposal

• Make type field mandatory on nearcore JSON RPC error
• Make type mandatory in TypedError class (near-api-js)
• Handle Failure status at the same level as other errors (in sendJsonRpc method)
• add errorPath field to the TypedError class or use existing data field
• add additional info based on error type, for example: shouldRetry: boolean, showToTheUser, expected: boolean
• Delete all error classes from near-api-js (except TypedError)

Open questions

1. How to save error context, options:

• CodeDoesNotExist.CompilationError.FunctionCallError.ActionError.TxExecutionError.ServerError
• "data":{"TxExecutionError":{"InvalidTxError":{"InvalidNonce":{"tx_nonce":2,"ak_nonce":28}}}}}
• just forget about it

2. How to save error data?

• as an Object
• ErrorContext with all possible error fields
• Unions
• Other?

3. Breaking changes, what is the best way to introduce them?

4. Should we? add any additional info to the error?

• shouldRetry: boolean
• showToTheUser: boolean
• expected: boolean

cc @frol @khorolets

[volovyks]

Some results from near-api-js Sync

cc @mikedotexe @chadoh @evgenykuzyakov @vgrichina

How to save error context? Proposals:
// Eugene

A.ActionError.InvalidNonce
{
"ActionError": {"action_index": 1},
"InvalidNone", {"tx_nonce": 2, "ak_nonce": 28},
}

// Clarification from Chad:

TypedError {
  ...etc,
  details: {
    errorPath: "CodeDoesNotExist.CompilationError.FunctionCallError.ActionError.TxExecutionError.ServerError",
    errorData: {
      ActionError: { step: 1 },
      InvalidNonce: { "tx_nonce": 2, "ak_nonce": 28 }
    }

Breaking changes:

• Refactor near-api-js → 1.0 → update near-core→ use new errors in near-api-js

Additional data:

• shouldRetry: boolean - check if we need this, real examples
• showToTheUser: boolean - check if we need this, real examples
• expected: boolean - check if we need this, real examples

Other

• Move all the info to GitHub discussions
• Get feedback from the wallet team

[volovyks]

Results of the meeting with Riqi (Paras):

• Paras is using near-api-js as well as JSON RPC directly. That is why consistency in error type fields are important for both JSON RPC and near-api-js.
• errorPath is not that important, it would be nice to hide it in details or context structure.
• errorPath with dots looks cleaner, but it's not important, since we do not need it in 99% of cases
• error data representation proposed by Eugene above appeared to be confusing at the first glance, but again, it used in ~1% of the cases
• we have not come up with any usable scenarios for shouldRetry, or other additional fields. It's better to have a clean error structure and rely on error type.

[volovyks]

Another idea that came up in the discussion, is to give users advice on why such an error usually occurs. It can be implemented as a new field (error.advice?), or as part of the error.message but in this case, it would be a breaking change.

One more idea is origin field, that will show where the error actually accused (contract, nearcore, near-api-js)

[volovyks]

The result from the call with @frol :

• All errors from RPC will be divided into 2 types: RPCErrors and ReceiptExecutionFailures.
• Both of them should be thrown early from sendJsonRpc.
• ReceiptExecutionFailure should use a recursive search to find out if an error exists.
• Both RPCError and ReceiptExecutionFailure should have a simple common interface (like TypedError) to be easily handled by developers.
• This changes should be added to New error handling design #507 and be merged together.