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.

Sign up for GitHub

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

Catch PastHorizon Exception and return 400 with descriptive Error #2059

Merged
merged 3 commits into from
Aug 27, 2020
+25 −4
Merged

Catch PastHorizon Exception and return 400 with descriptive Error #2059

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
05bd695
Catch PastHorizon Exception and return 400 with descriptive Error
Aug 24, 2020
ed1c160
Use 503 for PastHorizonException
Aug 24, 2020
8f09068
Line length fix
hasufell Aug 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions lib/core/src/Cardano/Wallet.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,11 @@ import Cardano.Wallet.Primitive.Model
, updateState
)
import Cardano.Wallet.Primitive.Slotting
( TimeInterpreter, slotRangeFromTimeRange, startTime )
( PastHorizonException (..)
, TimeInterpreter
, slotRangeFromTimeRange
, startTime
)
import Cardano.Wallet.Primitive.SyncProgress
( SyncProgress, SyncTolerance (..), syncProgress )
import Cardano.Wallet.Primitive.Types
Expand Down Expand Up @@ -327,7 +331,7 @@ import Cardano.Wallet.Transaction
import Cardano.Wallet.Unsafe
( unsafeXPrv )
import Control.Exception
( Exception )
( Exception, try )
import Control.Monad
( forM, forM_, replicateM, unless, when )
import Control.Monad.IO.Class
Expand Down Expand Up @@ -1838,7 +1842,11 @@ listTransactions ctx wid mMinWithdrawal mStart mEnd order = db & \DBLayer{..} ->
let err = ErrStartTimeLaterThanEndTime start end
throwE (ErrListTransactionsStartTimeLaterThanEndTime err)
_ -> do
liftIO $ ti $ slotRangeFromTimeRange $ Range mStart mEnd
liftIO (try $ ti $ slotRangeFromTimeRange $ Range mStart mEnd) >>=
\case
Left e@(PastHorizon{}) -> throwE (ErrListTransactionsPastHorizonException e)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exhaustive https://input-output-hk.github.io/ouroboros-network/ouroboros-consensus/Ouroboros-Consensus-HardFork-History-Summary.html#t:PastHorizonException

hasufell marked this conversation as resolved.
Show resolved Hide resolved
Right r -> pure r


-- | Get transaction and metadata from history for a given wallet.
getTransaction
Expand Down Expand Up @@ -2196,7 +2204,8 @@ data ErrListTransactions
= ErrListTransactionsNoSuchWallet ErrNoSuchWallet
| ErrListTransactionsStartTimeLaterThanEndTime ErrStartTimeLaterThanEndTime
| ErrListTransactionsMinWithdrawalWrong
deriving (Show, Eq)
| ErrListTransactionsPastHorizonException PastHorizonException
deriving (Show)

-- | Errors that can occur when trying to get transaction.
data ErrGetTransaction
Expand Down
10 changes: 10 additions & 0 deletions lib/core/src/Cardano/Wallet/Api/Server.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2312,6 +2312,7 @@ instance LiftHandler ErrListTransactions where
ErrListTransactionsMinWithdrawalWrong ->
apiError err400 MinWithdrawalWrong
"The minimum withdrawal value must be at least 1 Lovelace."
ErrListTransactionsPastHorizonException e -> handler e

instance LiftHandler ErrStartTimeLaterThanEndTime where
handler err = apiError err400 StartTimeLaterThanEndTime $ mconcat
Expand All @@ -2322,6 +2323,15 @@ instance LiftHandler ErrStartTimeLaterThanEndTime where
, "'."
]

instance LiftHandler PastHorizonException where
handler _ = apiError err503 PastHorizon $ mconcat
[ "Tried to convert something that is past the horizon"
, " (due to uncertainty about the next hard fork)."
, " Wait for the node to finish syncing to the hard fork."
, " Depending on the blockchain, this process can take an"
, " unknown amount of time."
]
Copy link
Contributor Author

@hasufell hasufell Aug 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Should the eras be listed? (Note that it's a list of Eras)

-- | We tried to convert something that is past the horizon
--
-- That is, we tried to convert something that is past the point in time
-- beyond which we lack information due to uncertainty about the next
-- hard fork.
--
-- We record the condition we were looking for and the bounds on the summary.
data PastHorizonException = PastHorizon CallStack [EraSummary]

data EraSummary = EraSummary {
      eraStart  :: !Bound     -- ^ Inclusive lower bound
    , eraEnd    :: !EraEnd    -- ^ Exclusive upper bound
    , eraParams :: !EraParams -- ^ Active parameters
    }
  deriving stock    (Show, Eq, Generic)
  deriving anyclass (NoUnexpectedThunks)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine like this already.


instance LiftHandler ErrGetTransaction where
handler = \case
ErrGetTransactionNoSuchWallet e -> handler e
Expand Down
1 change: 1 addition & 0 deletions lib/core/src/Cardano/Wallet/Api/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ data ApiErrorCode
| MinWithdrawalWrong
| AlreadyWithdrawing
| WithdrawalNotWorth
| PastHorizon
deriving (Eq, Generic, Show)

-- | Defines a point in time that can be formatted as and parsed from an
Expand Down