Revise API error codes

[piotr-iohk]

Issue Number

ADP-291

Overview

• f533091
  Revise error codes: - 404 when wallet not found - 403 when transaction is too big

• 430a31f
  Update swagger

• e90b264
  Update integration tests

Comments

Whie reviewing #2258 I noticed that some error codes are a bit inconsistent, e.g.:

• 410 or 404 is returned when the wallet is not found.
• 400 or 403 - when transaction is too big.

This pr is about to revise it and:

• return 404 on wallet not found (removing 410 altogether)
• return 403 on transaction too big

[hasufell]

Did you consider dropping 415 altogether?

[hasufell]

Not sure, I think this might actually be https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/413

[KtorZ]

Sounds relevant for this use case indeed.

[piotr-iohk]

Judging by the error message I'd say yes and no.

Yes, in case where the metadata is too big. As error message suggests:

Alternatively, try sending to less recipients or with smaller metadata.

Not necessarily, if the transaction turns out to be too big:

Note that I am selecting inputs randomly, so retrying *may work* 

For the sake of keeping it simple I'd stay with 403, which is more general.

[hasufell]

Ok, so Gone would be a viable option if we know the wallet existed beforehand. But I guess we don't?

[hasufell]

So, all uses of this type are as follows

    withRootKey @_ @s ctx wid pwd ErrSignPaymentWithRootKey $ \xprv scheme -> do
        let pwdP = preparePassphrase scheme pwd
        mapExceptT atomically $ do
            cp <- withExceptT ErrSignPaymentNoSuchWallet $ withNoSuchWallet wid $
                readCheckpoint (PrimaryKey wid)
            (cs', s') <- assignChangeAddressesForSelection
                argGenChange cs (getState cp)
            withExceptT ErrSignPaymentNoSuchWallet $
                putCheckpoint (PrimaryKey wid) (updateState s' cp)

            let keyFrom = isOwned (getState cp) (xprv, pwdP)
            let rewardAcnt = mkRewardAccount (xprv, pwdP)

            (tx, sealedTx) <- withExceptT ErrSignPaymentMkTx $ ExceptT $
                pure $ mkStdTx tl rewardAcnt keyFrom txExp md cs'

            (time, meta) <- liftIO $ mkTxMeta ti (currentTip cp) s' tx cs' txExp
            return (tx, meta, time, sealedTx)

That means we can only hit this code-path if we found a root key for the given wallet id. Does that imply the wallet existed?

[KtorZ]

Gone made sense in the context of concurrent request. When one creates a transaction, there are multiple back-and-forth with the database in separate db transactions. In principle, the wallet could disappear between any of these db transaction might there be a concurrent request deleting the wallet at the same time.

Arguably, this is veeeeeeeery edgy, and from a user perspective doesn't matter much. So changing to 404 sounds like a good move.

[piotr-iohk]

Did you consider dropping 415 altogether?

I haven't actually. As far as I see there are some tests against 415 in the test suite, so maybe let's have it here.

[piotr-iohk]

bors r+

[piotr-iohk]

bors r-

[iohk-bors]

Merge conflict.

[piotr-iohk]

bors r+

[iohk-bors]

Build succeeded:

• ci/hydra-build:required
• buildkite/cardano-wallet