From 474863e8ba40ced593b3d8be2becc061a6e99154 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 4 Nov 2021 10:49:05 +1100 Subject: [PATCH 1/6] Initialise the `RELEASE_BRANCH` envvar to ensure releasing works Thanks to @scratchscratchscratchy for pointing this out! --- .github/workflows/draft-new-release.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/draft-new-release.yml b/.github/workflows/draft-new-release.yml index ccbd701..294ef75 100644 --- a/.github/workflows/draft-new-release.yml +++ b/.github/workflows/draft-new-release.yml @@ -11,13 +11,15 @@ jobs: draft-new-release: name: "Draft a new release" runs-on: ubuntu-latest + env: + RELEASE_BRANCH: release/${{ github.event.inputs.version }} steps: - uses: actions/checkout@v2.3.5 with: token: ${{ secrets.BOTTY_GITHUB_TOKEN }} - name: Create release branch - run: git checkout -b release/${{ github.event.inputs.version }} + run: git checkout -b ${{ env.RELEASE_BRANCH }} - name: Initialize mandatory git config run: | From f8075fe5ca41a5a45a8f773121dc5b91d8b7d416 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Thu, 4 Nov 2021 12:04:13 +1030 Subject: [PATCH 2/6] Provide feedback about taker actions in HTTP response Invoke user actions (cfd actions & taking an order) synchronously in order to be able to communicate the results. Use HttpApiProblem to send error details to the frontend in a standard way. --- daemon/src/routes_taker.rs | 59 ++++++++++++++++++++++---------------- daemon/src/taker_cfd.rs | 12 ++++---- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/daemon/src/routes_taker.rs b/daemon/src/routes_taker.rs index 9f0d2e3..d383510 100644 --- a/daemon/src/routes_taker.rs +++ b/daemon/src/routes_taker.rs @@ -4,6 +4,7 @@ use daemon::model::{Leverage, Price, Usd, WalletInfo}; use daemon::routes::EmbeddedFileExt; use daemon::to_sse_event::{CfdAction, CfdsWithAuxData, ToSseEvent}; use daemon::{bitmex_price_feed, taker_cfd}; +use http_api_problem::{HttpApiProblem, StatusCode}; use rocket::http::{ContentType, Status}; use rocket::response::stream::EventStream; use rocket::response::{status, Responder}; @@ -15,7 +16,7 @@ use std::borrow::Cow; use std::path::PathBuf; use tokio::select; use tokio::sync::watch; -use xtra::prelude::MessageChannel; +use xtra::prelude::*; #[rocket::get("/feed")] pub async fn feed( @@ -105,13 +106,21 @@ pub struct CfdOrderRequest { pub async fn post_order_request( cfd_order_request: Json, take_offer_channel: &State>>, -) { +) -> Result, HttpApiProblem> { take_offer_channel - .do_send(taker_cfd::TakeOffer { + .send(taker_cfd::TakeOffer { order_id: cfd_order_request.order_id, quantity: cfd_order_request.quantity, }) - .expect("actor to always be available"); + .await + .unwrap_or_else(|e| anyhow::bail!(e.to_string())) + .map_err(|e| { + HttpApiProblem::new(StatusCode::INTERNAL_SERVER_ERROR) + .title("Order request failed") + .detail(e.to_string()) + })?; + + Ok(status::Accepted(None)) } #[rocket::post("/cfd//")] @@ -120,37 +129,37 @@ pub async fn post_cfd_action( action: CfdAction, cfd_action_channel: &State>>, quote_updates: &State>, -) -> Result, status::BadRequest> { +) -> Result, HttpApiProblem> { use taker_cfd::CfdAction::*; - match action { + let result = match action { CfdAction::AcceptOrder | CfdAction::RejectOrder | CfdAction::AcceptSettlement | CfdAction::RejectSettlement | CfdAction::AcceptRollOver | CfdAction::RejectRollOver => { - return Err(status::BadRequest(None)); - } - CfdAction::Commit => { - cfd_action_channel - .do_send(Commit { order_id: id }) - .map_err(|e| status::BadRequest(Some(e.to_string())))?; + return Err(HttpApiProblem::new(StatusCode::BAD_REQUEST) + .detail("taker cannot invoke this actions")); } + CfdAction::Commit => cfd_action_channel.send(Commit { order_id: id }), CfdAction::Settle => { let current_price = quote_updates.borrow().for_taker(); - cfd_action_channel - .do_send(ProposeSettlement { - order_id: id, - current_price, - }) - .expect("actor to always be available"); + cfd_action_channel.send(ProposeSettlement { + order_id: id, + current_price, + }) } - CfdAction::RollOver => { - cfd_action_channel - .do_send(ProposeRollOver { order_id: id }) - .expect("actor to always be available"); - } - } + CfdAction::RollOver => cfd_action_channel.send(ProposeRollOver { order_id: id }), + }; + + result + .await + .unwrap_or_else(|e| anyhow::bail!(e.to_string())) + .map_err(|e| { + HttpApiProblem::new(StatusCode::INTERNAL_SERVER_ERROR) + .title(action.to_string() + " failed") + .detail(e.to_string()) + })?; Ok(status::Accepted(None)) } @@ -177,7 +186,7 @@ pub struct MarginResponse { #[rocket::post("/calculate/margin", data = "")] pub fn margin_calc( margin_request: Json, -) -> Result>, status::BadRequest> { +) -> Result>, HttpApiProblem> { let margin = calculate_long_margin( margin_request.price, margin_request.quantity, diff --git a/daemon/src/taker_cfd.rs b/daemon/src/taker_cfd.rs index 89f94cc..b059963 100644 --- a/daemon/src/taker_cfd.rs +++ b/daemon/src/taker_cfd.rs @@ -652,8 +652,8 @@ where #[async_trait] impl Handler for Actor { - async fn handle(&mut self, msg: TakeOffer, _ctx: &mut Context) { - log_error!(self.handle_take_offer(msg.order_id, msg.quantity)); + async fn handle(&mut self, msg: TakeOffer, _ctx: &mut Context) -> Result<()> { + self.handle_take_offer(msg.order_id, msg.quantity).await } } @@ -664,7 +664,7 @@ where + xtra::Handler + xtra::Handler, { - async fn handle(&mut self, msg: CfdAction, _ctx: &mut Context) { + async fn handle(&mut self, msg: CfdAction, _ctx: &mut Context) -> Result<()> { use CfdAction::*; if let Err(e) = match msg { @@ -679,7 +679,9 @@ where ProposeRollOver { order_id } => self.handle_propose_roll_over(order_id).await, } { tracing::error!("Message handler failed: {:#}", e); + anyhow::bail!(e) } + Ok(()) } } @@ -789,11 +791,11 @@ where } impl Message for TakeOffer { - type Result = (); + type Result = Result<()>; } impl Message for CfdAction { - type Result = (); + type Result = Result<()>; } // this signature is a bit different because we use `Address::attach_stream` From 06af1e42054837aceb9df2d692a3b1c5ea622c7e Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Thu, 4 Nov 2021 12:05:05 +1030 Subject: [PATCH 3/6] Clean up errors handling in maker's HTTP response Populate details about the problem if they're available. --- daemon/src/routes_maker.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/daemon/src/routes_maker.rs b/daemon/src/routes_maker.rs index ed68ae3..86140ef 100644 --- a/daemon/src/routes_maker.rs +++ b/daemon/src/routes_maker.rs @@ -122,11 +122,11 @@ pub async fn post_sell_order( max_quantity: order.max_quantity, }) .await - .unwrap_or_else(|_| anyhow::bail!("actor disconnected")) // TODO: is there a better way? - .map_err(|_| { + .unwrap_or_else(|e| anyhow::bail!(e)) + .map_err(|e| { HttpApiProblem::new(StatusCode::INTERNAL_SERVER_ERROR) - .title("Action failed") - .detail("failed to post a sell order") + .title("Posting offer failed") + .detail(e.to_string()) })?; Ok(status::Accepted(None)) @@ -179,10 +179,11 @@ pub async fn post_cfd_action( result .await - .unwrap_or_else(|_| anyhow::bail!("actor disconnected")) // TODO: is there a better way? - .map_err(|_| { + .unwrap_or_else(|e| anyhow::bail!(e)) + .map_err(|e| { HttpApiProblem::new(StatusCode::INTERNAL_SERVER_ERROR) .title(action.to_string() + " failed") + .detail(e.to_string()) })?; Ok(status::Accepted(None)) From 89f28fca7ffedc06fcfb73070dafa822d6ae0c47 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Thu, 4 Nov 2021 12:08:59 +1030 Subject: [PATCH 4/6] Display error toast from failed actions from CfdTable This code concerns both maker and taker UIs. --- frontend/src/TakerApp.tsx | 15 +++++++++------ frontend/src/components/cfdtables/CfdTable.tsx | 18 ++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/frontend/src/TakerApp.tsx b/frontend/src/TakerApp.tsx index 7879bc6..ab08820 100644 --- a/frontend/src/TakerApp.tsx +++ b/frontend/src/TakerApp.tsx @@ -21,6 +21,7 @@ import { useEventSource } from "react-sse-hooks"; import { CfdTable } from "./components/cfdtables/CfdTable"; import CurrencyInputField from "./components/CurrencyInputField"; import useLatestEvent from "./components/Hooks"; +import { HttpError } from "./components/HttpError"; import { Cfd, intoCfd, intoOrder, Order, StateGroupKey, WalletInfo } from "./components/Types"; import Wallet from "./components/Wallet"; @@ -43,7 +44,8 @@ async function postCfdOrderRequest(payload: CfdOrderRequestPayload) { let res = await fetch(`/api/cfd/order`, { method: "POST", body: JSON.stringify(payload) }); if (!res.status.toString().startsWith("2")) { - throw new Error("failed to create new CFD order request: " + res.status + ", " + res.statusText); + const resp = await res.json(); + throw new HttpError(resp); } } @@ -51,7 +53,8 @@ async function getMargin(payload: MarginRequestPayload): Promise let res = await fetch(`/api/calculate/margin`, { method: "POST", body: JSON.stringify(payload) }); if (!res.status.toString().startsWith("2")) { - throw new Error("failed to create new CFD order request: " + res.status + ", " + res.statusText); + const resp = await res.json(); + throw new HttpError(resp); } return res.json(); @@ -106,10 +109,10 @@ export default function App() { }; calculateMargin(payload); }, // Eslint demands us to include `calculateMargin` in the list of dependencies. - // We don't want that as we will end up in an endless loop. It is safe to ignore `calculateMargin` because - // nothing in `calculateMargin` depends on outside values, i.e. is guaranteed to be stable. - // eslint-disable-next-line react-hooks/exhaustive-deps - [margin, effectiveQuantity, order]); + // We don't want that as we will end up in an endless loop. It is safe to ignore `calculateMargin` because + // nothing in `calculateMargin` depends on outside values, i.e. is guaranteed to be stable. + // eslint-disable-next-line react-hooks/exhaustive-deps + [margin, effectiveQuantity, order]); const format = (val: any) => `$` + val; const parse = (val: any) => val.replace(/^\$/, ""); diff --git a/frontend/src/components/cfdtables/CfdTable.tsx b/frontend/src/components/cfdtables/CfdTable.tsx index cb91270..53923f2 100644 --- a/frontend/src/components/cfdtables/CfdTable.tsx +++ b/frontend/src/components/cfdtables/CfdTable.tsx @@ -31,6 +31,8 @@ import { import React from "react"; import { useAsync } from "react-async"; import { Column, Row, useExpanded, useSortBy, useTable } from "react-table"; +import createErrorToast from "../ErrorToast"; +import { HttpError } from "../HttpError"; import Timestamp from "../Timestamp"; import { Action, Cfd } from "../Types"; @@ -48,15 +50,7 @@ export function CfdTable( try { await doPostAction(orderId, action); } catch (e) { - const description = typeof e === "string" ? e : JSON.stringify(e); - - toast({ - title: "Error", - description, - status: "error", - duration: 9000, - isClosable: true, - }); + createErrorToast(toast, e); } }, }); @@ -400,8 +394,12 @@ export function Table({ columns, tableData, hiddenColumns, renderDetails }: Tabl } async function doPostAction(id: string, action: string) { - await fetch( + let res = await fetch( `/api/cfd/${id}/${action}`, { method: "POST", credentials: "include" }, ); + if (!res.status.toString().startsWith("2")) { + const resp = await res.json(); + throw new HttpError(resp); + } } From fa46ae15e52ab6eebe45659285ca38af67f34e1e Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Thu, 4 Nov 2021 12:09:28 +1030 Subject: [PATCH 5/6] Display error toasts from failed actions in the taker --- frontend/src/TakerApp.tsx | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/frontend/src/TakerApp.tsx b/frontend/src/TakerApp.tsx index ab08820..c50af4c 100644 --- a/frontend/src/TakerApp.tsx +++ b/frontend/src/TakerApp.tsx @@ -20,6 +20,7 @@ import { useAsync } from "react-async"; import { useEventSource } from "react-sse-hooks"; import { CfdTable } from "./components/cfdtables/CfdTable"; import CurrencyInputField from "./components/CurrencyInputField"; +import createErrorToast from "./components/ErrorToast"; import useLatestEvent from "./components/Hooks"; import { HttpError } from "./components/HttpError"; import { Cfd, intoCfd, intoOrder, Order, StateGroupKey, WalletInfo } from "./components/Types"; @@ -84,15 +85,7 @@ export default function App() { let res = await getMargin(payload as MarginRequestPayload); setMargin(res.margin.toString()); } catch (e) { - const description = typeof e === "string" ? e : JSON.stringify(e); - - toast({ - title: "Error", - description, - status: "error", - duration: 9000, - isClosable: true, - }); + createErrorToast(toast, e); } }, }); @@ -109,10 +102,10 @@ export default function App() { }; calculateMargin(payload); }, // Eslint demands us to include `calculateMargin` in the list of dependencies. - // We don't want that as we will end up in an endless loop. It is safe to ignore `calculateMargin` because - // nothing in `calculateMargin` depends on outside values, i.e. is guaranteed to be stable. - // eslint-disable-next-line react-hooks/exhaustive-deps - [margin, effectiveQuantity, order]); + // We don't want that as we will end up in an endless loop. It is safe to ignore `calculateMargin` because + // nothing in `calculateMargin` depends on outside values, i.e. is guaranteed to be stable. + // eslint-disable-next-line react-hooks/exhaustive-deps + [margin, effectiveQuantity, order]); const format = (val: any) => `$` + val; const parse = (val: any) => val.replace(/^\$/, ""); @@ -122,15 +115,7 @@ export default function App() { try { await postCfdOrderRequest(payload as CfdOrderRequestPayload); } catch (e) { - const description = typeof e === "string" ? e : JSON.stringify(e); - - toast({ - title: "Error", - description, - status: "error", - duration: 9000, - isClosable: true, - }); + createErrorToast(toast, e); } }, }); From a591c166e892fdd76a0b31805de6541bdc926ce3 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Thu, 4 Nov 2021 17:33:13 +1030 Subject: [PATCH 6/6] Update daemon/src/routes_taker.rs Co-authored-by: Daniel Karzel --- daemon/src/routes_taker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/src/routes_taker.rs b/daemon/src/routes_taker.rs index d383510..3e3e66e 100644 --- a/daemon/src/routes_taker.rs +++ b/daemon/src/routes_taker.rs @@ -139,7 +139,7 @@ pub async fn post_cfd_action( | CfdAction::AcceptRollOver | CfdAction::RejectRollOver => { return Err(HttpApiProblem::new(StatusCode::BAD_REQUEST) - .detail("taker cannot invoke this actions")); + .detail(format!("taker cannot invoke action {}", action))); } CfdAction::Commit => cfd_action_channel.send(Commit { order_id: id }), CfdAction::Settle => {