Make the `NotEnoughMoney` API error machine-readable. #4348

jonathanknowles · 2023-12-15T08:36:37Z

Issue

Description

This PR revises the NotEnoughMoney API error so that it is machine-readable, with a well-defined schema.

Example

{ "code": "not_enough_money"
, "message": "I can't process this payment as there are not enough funds available in the wallet."
, "info":
  { "shortfall":
    { "ada":
      { "quantity": 1
      , "unit": "lovelace"
      }
    , "assets":
      [ { "policy_id": "FRUIT"
        , "asset_name": "APPLE"
        , "quantity": 2
        }
      , { "policy_id": "FRUIT"
        , "asset_name": "BANANA"
        , "quantity": 4
        }
      ]
    }
  }
}

## Summary This PR forks the HTTP API error `not_enough_money` into two separate errors: - `not_enough_money` for when there **_really_** isn't enough money - `no_utxos_available` for when there **_might_** be enough money, but there are no UTxOs available These two cases can require subtly different workarounds from the user. ## Details This PR solves two problems. ### Problem 1 The `not_enough_money` error is **_inappropriate_** in the case where the reward balance alone would be sufficient to cover the amount required. For example, suppose a user has a wallet with: - no UTxOs; - an unspent reward balance of `1,000,000,000` ada. Suppose that the user attempts to make a payment transaction of `10` ada. In this case, the attempt will fail because: - every transaction is required to spend at least one UTxO; but - the wallet has no UTxOs available. But the user will receive a misleading `not_enough_money` error, even though they have more than enough money. This PR solves this problem by always using `no_utxos_available` when there are no UTxOs. ### Problem 2 In the case where UTxOs **_are_** available, but their value is insufficient, we currently attempt to embed information about the shortfall within the error message string: > ``` > I can't process this payment as there are not enough funds in the wallet. I am missing: > - lovelace: 123,456 lovelace > - assets: ... <information embedded within string> > ``` This information is valuable for UI clients that wish to inform users how much extra value they would need, but the information is tricky for UI clients to parse, as it's embedded within a **_string_**. We wish to turn the shortfall case into a **_machine-readable_** error with a specified JSON encoding (see #4348). However, this error would not be appropriate for the case where there are no UTxOs available at all, as there would not necessarily even be a "shortfall". This PR avoids this problem by giving the former case (no UTxOs available) its own dedicated error that will not be affected by #4348.

…This PR forks the HTTP API error `not_enough_money` into two separate errors: - `not_enough_money` for when there **_really_** isn't enough money - `no_utxos_available` for when there **_might_** be enough money, but there are no UTxOs available These two cases can require subtly different workarounds from the user. ## Details This PR solves two problems. ### Problem 1 The `not_enough_money` error is **_inappropriate_** in the case where the reward balance alone would be sufficient to cover the amount required. For example, suppose a user has a wallet with: - no UTxOs; - an unspent reward balance of `1,000,000,000` ada. Suppose that the user attempts to make a payment transaction of `10` ada. In this case, the attempt will fail because: - every transaction is required to spend at least one UTxO; but - the wallet has no UTxOs available. But the user will receive a misleading `not_enough_money` error, even though they have more than enough money. This PR solves this problem by always using `no_utxos_available` when there are no UTxOs. ### Problem 2 In the case where UTxOs **_are_** available, but their value is insufficient, we currently attempt to embed information about the shortfall within the error message string: > ``` > I can't process this payment as there are not enough funds in the wallet. I am missing: > - lovelace: 123,456 lovelace > - assets: ... <information embedded within string> > ``` This information is valuable for UI clients that wish to inform users how much extra value they would need, but the information is tricky for UI clients to parse, as it's embedded within a **_string_**. We wish to turn the shortfall case into a **_machine-readable_** error with a specified JSON encoding (see #4348). However, this error would not be appropriate for the case where there are no UTxOs available at all, as there would not necessarily even be a "shortfall". This PR avoids this problem by giving the former case (no UTxOs available) its own dedicated error that will not be affected by #4348. Source commit: 15f75af

We can verify that the `ApiErrorNotEnoughMoney` constructor has an info field.

…ion.

jonathanknowles · 2023-12-22T09:05:02Z

specifications/api/swagger.yaml

-      description: May occur when there's not enough money in the wallet to cover a requested payment.
+      description: |
+        Indicates that there's not enough money in the wallet to cover a
+        requested payment.


Add a sentence that describes the shortfall field in more detail.
Emphasise that the shortfall just describes the assets that are missing, and that if supplied, would unblock whatever operation it was that the user was attempting.

Fixed in 0432db5.

In response to review feedback: #4348 (comment)

paweljakubas · 2023-12-22T09:52:32Z

lib/wallet/integration/src/Test/Integration/Framework/TestData.hs

@@ -378,11 +377,6 @@ errMsg403NotAnIcarusWallet =
    "I cannot derive new address for this wallet type.\
    \ Make sure to use a sequential wallet style, like Icarus."

-errMsg403NotEnoughMoney :: String


Instead of inspecting the `message` string.

paweljakubas · 2023-12-22T09:54:38Z

lib/wallet/integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs

-            , expectErrorMessage errMsg403NotEnoughMoney
-            ]
+        verify rTx [expectResponseCode HTTP.status403]
+        decodeErrorInfo rTx `shouldSatisfy` \case


paweljakubas

very nice PR

…# Issue ADP-3247 ## Description This PR revises the `NotEnoughMoney` API error so that it is machine-readable, with a well-defined schema. ## Example ```json { "code": "not_enough_money" , "message": "I can't process this payment as there are not enough funds available in the wallet." , "info": { "shortfall": { "ada": { "quantity": 1 , "unit": "lovelace" } , "assets": [ { "policy_id": "FRUIT" , "asset_name": "APPLE" , "quantity": 2 } , { "policy_id": "FRUIT" , "asset_name": "BANANA" , "quantity": 4 } ] } } } ``` Source commit: 4335f75

jonathanknowles self-assigned this Dec 15, 2023

jonathanknowles added DO NOT MERGE EXPERIMENTAL labels Dec 15, 2023

jonathanknowles changed the title ~~WIP: Make NotEnoughMoney a machine-readable error.~~ WIP: Make NotEnoughMoney a machine-readable API error. Dec 15, 2023

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch 3 times, most recently from 91320a2 to 68a9d4d Compare December 19, 2023 07:10

jonathanknowles changed the base branch from master to jonathanknowles/api-amount December 19, 2023 11:27

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch 2 times, most recently from fa71b0e to 95f7f35 Compare December 19, 2023 11:41

jonathanknowles force-pushed the jonathanknowles/api-amount branch from 98f88e8 to 986b80c Compare December 20, 2023 02:43

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch 2 times, most recently from aca2498 to 1ab2d97 Compare December 20, 2023 02:53

jonathanknowles force-pushed the jonathanknowles/api-amount branch 2 times, most recently from 36ef3c9 to de9165b Compare December 20, 2023 04:33

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch from 1ab2d97 to 9e4ce91 Compare December 20, 2023 04:34

jonathanknowles force-pushed the jonathanknowles/api-amount branch from de9165b to 38ea1e2 Compare December 20, 2023 04:59

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch from 9e4ce91 to e903be4 Compare December 20, 2023 05:11

jonathanknowles changed the base branch from jonathanknowles/api-amount to jonathanknowles/no-utxos-available December 20, 2023 05:11

jonathanknowles mentioned this pull request Dec 20, 2023

Fork the not_enough_money HTTP API error. #4366

Merged

jonathanknowles changed the title ~~WIP: Make NotEnoughMoney a machine-readable API error.~~ Make NotEnoughMoney a machine-readable API error. Dec 20, 2023

jonathanknowles removed DO NOT MERGE EXPERIMENTAL labels Dec 20, 2023

Base automatically changed from jonathanknowles/no-utxos-available to master December 20, 2023 09:39

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch 2 times, most recently from 5b3937b to be2e18c Compare December 21, 2023 02:43

jonathanknowles marked this pull request as ready for review December 21, 2023 04:10

jonathanknowles marked this pull request as draft December 21, 2023 04:11

jonathanknowles marked this pull request as ready for review December 21, 2023 07:44

jonathanknowles changed the title ~~Make NotEnoughMoney a machine-readable API error.~~ Make the NotEnoughMoney API error machine-readable. Dec 21, 2023

jonathanknowles marked this pull request as draft December 21, 2023 11:02

jonathanknowles marked this pull request as ready for review December 22, 2023 00:13

jonathanknowles added 9 commits December 22, 2023 01:44

Add new Haskell data type ApiErrorNotEnoughMoney.

0e230ba

Add JSON encoding test for type ApiErrorNotEnoughMoney.

8dae80a

Parameterise NotEnoughMoney by ApiErrorNotEnoughMoney.

27290a3

Remove detail from message string for NotEnoughMoney error.

185fb9a

Regenerate ApiError.json golden file.

8744e45

We can verify that the `ApiErrorNotEnoughMoney` constructor has an info field.

Expect NotEnoughMoney error in integration tests.

6300260

Remove redundant errMsg403NotEnoughMoney test string.

1fda96d

Revise description of not_enough_money error in HTTP API specificat…

5c69fba

…ion.

Add schema for info field of API error not_enough_money.

885a999

jonathanknowles force-pushed the jonathanknowles/machine-readable-not-enough-money branch from be2e18c to 885a999 Compare December 22, 2023 01:44

jonathanknowles commented Dec 22, 2023

View reviewed changes

jonathanknowles requested review from paweljakubas and HeinrichApfelmus December 22, 2023 09:27

Add a description of the shortfall object in errNotEnoughMoney.

0432db5

In response to review feedback: #4348 (comment)

paweljakubas reviewed Dec 22, 2023

View reviewed changes

Fix E2E tests to inspect shortfall field of not_enough_money error.

13b4a73

Instead of inspecting the `message` string.

paweljakubas reviewed Dec 22, 2023

View reviewed changes

paweljakubas approved these changes Dec 22, 2023

View reviewed changes

jonathanknowles enabled auto-merge December 22, 2023 09:59

jonathanknowles added this pull request to the merge queue Dec 22, 2023

Merged via the queue into master with commit 4335f75 Dec 22, 2023
4 checks passed

jonathanknowles deleted the jonathanknowles/machine-readable-not-enough-money branch December 22, 2023 10:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the `NotEnoughMoney` API error machine-readable. #4348

Make the `NotEnoughMoney` API error machine-readable. #4348

jonathanknowles commented Dec 15, 2023 •

edited

Loading

jonathanknowles Dec 22, 2023

jonathanknowles Dec 22, 2023

paweljakubas Dec 22, 2023

paweljakubas Dec 22, 2023

paweljakubas left a comment

Make the NotEnoughMoney API error machine-readable. #4348

Make the NotEnoughMoney API error machine-readable. #4348

Conversation

jonathanknowles commented Dec 15, 2023 • edited Loading

Issue

Description

Example

jonathanknowles Dec 22, 2023

Choose a reason for hiding this comment

jonathanknowles Dec 22, 2023

Choose a reason for hiding this comment

paweljakubas Dec 22, 2023

Choose a reason for hiding this comment

paweljakubas Dec 22, 2023

Choose a reason for hiding this comment

paweljakubas left a comment

Choose a reason for hiding this comment

Make the `NotEnoughMoney` API error machine-readable. #4348

Make the `NotEnoughMoney` API error machine-readable. #4348

jonathanknowles commented Dec 15, 2023 •

edited

Loading