[taler-wallet-core] 01/02: nicer HTTP helper in preparation for better error handling

[gnunet]

This is an automated email from the git hooks/post-receive script.

dold pushed a commit to branch master
in repository wallet-core.

commit dd2efc3d78f2dfda44f8182f9638723dcb839781
Author: Florian Dold <florian.dold@gmail.com>
AuthorDate: Mon Jul 20 17:46:49 2020 +0530

    nicer HTTP helper in preparation for better error handling
---
 integrationtests/common.sh |  24 +++++++++-
 src/headless/merchant.ts   |  11 ++---
 src/operations/errors.ts   |  58 -----------------------
 src/operations/pay.ts      |  43 +++++++++--------
 src/operations/recoup.ts   |  26 +++++-----
 src/operations/withdraw.ts |  12 +++--
 src/types/notifications.ts |   1 +
 src/util/http.ts           | 115 +++++++++++++++++++++++++++++++++++++++++++++
 src/util/taleruri.ts       |  11 +----
 9 files changed, 191 insertions(+), 110 deletions(-)

diff --git a/integrationtests/common.sh b/integrationtests/common.sh
index d228d1ea..d3f34767 100644
--- a/integrationtests/common.sh
+++ b/integrationtests/common.sh
@@ -1,8 +1,8 @@
 #!/bin/bash
 
-function setup_config() {
-    set -eu
+set -eu
 
+function setup_config() {
     echo -n "Testing for taler-bank-manage"
     taler-bank-manage -h >/dev/null </dev/null || exit_skip " MISSING"
     echo " FOUND"
@@ -135,11 +135,31 @@ function wait_for_services() {
     echo " DONE"
 }
 
+# Configure merchant instances
+function configure_merchant() {
+  json='
+    {
+      "id": "default",
+      "name": "GNU Taler Merchant",
+      "payto_uris": ["payto://x-taler-bank/test_merchant"],
+      "address": {},
+      "jurisdiction": {},
+      "default_max_wire_fee": "TESTKUDOS:1",
+      "default_wire_fee_amortization": 3,
+      "default_max_deposit_fee": "TESTKUDOS:1",
+      "default_wire_transfer_delay": {"d_ms": "forever"},
+      "default_pay_delay": {"d_ms": "forever"}
+    }
+  '
+  curl -v -XPOST --data "$json" "${MERCHANT_URL}private/instances"
+}
+
 function normal_start_and_wait() {
     setup_config "$1"
     setup_services
     launch_services
     wait_for_services
+    configure_merchant
 }
 
 # provide the service URL as first parameter
diff --git a/src/headless/merchant.ts b/src/headless/merchant.ts
index b479c5a8..3324924f 100644
--- a/src/headless/merchant.ts
+++ b/src/headless/merchant.ts
@@ -37,9 +37,8 @@ export class MerchantBackendConnection {
     reason: string,
     refundAmount: string,
   ): Promise<string> {
-    const reqUrl = new URL("refund", this.merchantBaseUrl);
+    const reqUrl = new URL(`private/orders/${orderId}/refund`, 
this.merchantBaseUrl);
     const refundReq = {
-      order_id: orderId,
       reason,
       refund: refundAmount,
     };
@@ -66,10 +65,11 @@ export class MerchantBackendConnection {
   constructor(public merchantBaseUrl: string, public apiKey: string) {}
 
   async authorizeTip(amount: string, justification: string): Promise<string> {
-    const reqUrl = new URL("tip-authorize", this.merchantBaseUrl).href;
+    const reqUrl = new URL("private/tips", this.merchantBaseUrl).href;
     const tipReq = {
       amount,
       justification,
+      next_url: "about:blank",
     };
     const resp = await axios({
       method: "post",
@@ -93,7 +93,7 @@ export class MerchantBackendConnection {
     fulfillmentUrl: string,
   ): Promise<{ orderId: string }> {
     const t = Math.floor(new Date().getTime() / 1000) + 15 * 60;
-    const reqUrl = new URL("order", this.merchantBaseUrl).href;
+    const reqUrl = new URL("private/orders", this.merchantBaseUrl).href;
     const orderReq = {
       order: {
         amount,
@@ -123,11 +123,10 @@ export class MerchantBackendConnection {
   }
 
   async checkPayment(orderId: string): Promise<CheckPaymentResponse> {
-    const reqUrl = new URL("check-payment", this.merchantBaseUrl).href;
+    const reqUrl = new URL(`private/orders/${orderId}`, 
this.merchantBaseUrl).href;
     const resp = await axios({
       method: "get",
       url: reqUrl,
-      params: { order_id: orderId },
       responseType: "json",
       headers: {
         Authorization: `ApiKey ${this.apiKey}`,
diff --git a/src/operations/errors.ts b/src/operations/errors.ts
index c8885c9e..9def02b0 100644
--- a/src/operations/errors.ts
+++ b/src/operations/errors.ts
@@ -53,64 +53,6 @@ export class OperationFailedError extends Error {
   }
 }
 
-/**
- * Process an HTTP response that we expect to contain Taler-specific JSON.
- *
- * Depending on the status code, we throw an exception.  This function
- * will try to extract Taler-specific error information from the HTTP response
- * if possible.
- */
-export async function scrutinizeTalerJsonResponse<T>(
-  resp: HttpResponse,
-  codec: Codec<T>,
-): Promise<T> {
-  // FIXME: We should distinguish between different types of error status
-  // to react differently (throttle, report permanent failure)
-
-  // FIXME: Make sure that when we receive an error message,
-  // it looks like a Taler error message
-
-  if (resp.status !== 200) {
-    let exc: OperationFailedError | undefined = undefined;
-    try {
-      const errorJson = await resp.json();
-      const m = `received error response (status ${resp.status})`;
-      exc = new OperationFailedError({
-        type: "protocol",
-        message: m,
-        details: {
-          httpStatusCode: resp.status,
-          errorResponse: errorJson,
-        },
-      });
-    } catch (e) {
-      const m = "could not parse response JSON";
-      exc = new OperationFailedError({
-        type: "network",
-        message: m,
-        details: {
-          status: resp.status,
-        },
-      });
-    }
-    throw exc;
-  }
-  let json: any;
-  try {
-    json = await resp.json();
-  } catch (e) {
-    const m = "could not parse response JSON";
-    throw new OperationFailedError({
-      type: "network",
-      message: m,
-      details: {
-        status: resp.status,
-      },
-    });
-  }
-  return codec.decode(json);
-}
-
 /**
  * Run an operation and call the onOpError callback
  * when there was an exception or operation error that must be reported.
diff --git a/src/operations/pay.ts b/src/operations/pay.ts
index a1619074..abcb2ad1 100644
--- a/src/operations/pay.ts
+++ b/src/operations/pay.ts
@@ -52,12 +52,13 @@ import {
 import * as Amounts from "../util/amounts";
 import { AmountJson } from "../util/amounts";
 import { Logger } from "../util/logging";
-import { getOrderDownloadUrl, parsePayUri } from "../util/taleruri";
+import { parsePayUri } from "../util/taleruri";
 import { guardOperationException } from "./errors";
 import { createRefreshGroup, getTotalRefreshCost } from "./refresh";
 import { InternalWalletState } from "./state";
 import { getTimestampNow, timestampAddDuration } from "../util/time";
 import { strcmp, canonicalJson } from "../util/helpers";
+import { httpPostTalerJson } from "../util/http";
 
 /**
  * Logger.
@@ -534,7 +535,9 @@ async function incrementProposalRetry(
     pr.lastError = err;
     await tx.put(Stores.proposals, pr);
   });
-  ws.notify({ type: NotificationType.ProposalOperationError });
+  if (err) {
+    ws.notify({ type: NotificationType.ProposalOperationError, error: err });
+  }
 }
 
 async function incrementPurchasePayRetry(
@@ -600,25 +603,25 @@ async function processDownloadProposalImpl(
     return;
   }
 
-  const parsedUrl = new URL(
-    getOrderDownloadUrl(proposal.merchantBaseUrl, proposal.orderId),
-  );
-  parsedUrl.searchParams.set("nonce", proposal.noncePub);
-  const urlWithNonce = parsedUrl.href;
-  console.log("downloading contract from '" + urlWithNonce + "'");
-  let resp;
-  try {
-    resp = await ws.http.get(urlWithNonce);
-  } catch (e) {
-    console.log("contract download failed", e);
-    throw e;
-  }
-
-  if (resp.status !== 200) {
-    throw Error(`contract download failed with status ${resp.status}`);
-  }
+  const orderClaimUrl = new URL(
+    `orders/${proposal.orderId}/claim`,
+    proposal.merchantBaseUrl,
+  ).href;
+  logger.trace("downloading contract from '" + orderClaimUrl + "'");
+
+  
+  const proposalResp = await httpPostTalerJson({
+    url: orderClaimUrl,
+    body: {
+      nonce: proposal.noncePub,
+    },
+    codec: codecForProposal(),
+    http: ws.http,
+  });
 
-  const proposalResp = codecForProposal().decode(await resp.json());
+  // The proposalResp contains the contract terms as raw JSON,
+  // as the coded to parse them doesn't necessarily round-trip.
+  // We need this raw JSON to compute the contract terms hash.
 
   const contractTermsHash = await ws.cryptoApi.hashString(
     canonicalJson(proposalResp.contract_terms),
diff --git a/src/operations/recoup.ts b/src/operations/recoup.ts
index e1c2325d..d1b3c3bd 100644
--- a/src/operations/recoup.ts
+++ b/src/operations/recoup.ts
@@ -48,7 +48,8 @@ import { RefreshReason, OperationError } from 
"../types/walletTypes";
 import { TransactionHandle } from "../util/query";
 import { encodeCrock, getRandomBytes } from "../crypto/talerCrypto";
 import { getTimestampNow } from "../util/time";
-import { guardOperationException, scrutinizeTalerJsonResponse } from 
"./errors";
+import { guardOperationException } from "./errors";
+import { httpPostTalerJson } from "../util/http";
 
 async function incrementRecoupRetry(
   ws: InternalWalletState,
@@ -146,11 +147,12 @@ async function recoupWithdrawCoin(
 
   const recoupRequest = await ws.cryptoApi.createRecoupRequest(coin);
   const reqUrl = new URL(`/coins/${coin.coinPub}/recoup`, 
coin.exchangeBaseUrl);
-  const resp = await ws.http.postJson(reqUrl.href, recoupRequest);
-  const recoupConfirmation = await scrutinizeTalerJsonResponse(
-    resp,
-    codecForRecoupConfirmation(),
-  );
+  const recoupConfirmation = await httpPostTalerJson({
+    url: reqUrl.href,
+    body: recoupRequest,
+    codec: codecForRecoupConfirmation(),
+    http: ws.http,
+  });
 
   if (recoupConfirmation.reserve_pub !== reservePub) {
     throw Error(`Coin's reserve doesn't match reserve on recoup`);
@@ -220,11 +222,13 @@ async function recoupRefreshCoin(
   const recoupRequest = await ws.cryptoApi.createRecoupRequest(coin);
   const reqUrl = new URL(`/coins/${coin.coinPub}/recoup`, 
coin.exchangeBaseUrl);
   console.log("making recoup request");
-  const resp = await ws.http.postJson(reqUrl.href, recoupRequest);
-  const recoupConfirmation = await scrutinizeTalerJsonResponse(
-    resp,
-    codecForRecoupConfirmation(),
-  );
+  
+  const recoupConfirmation = await httpPostTalerJson({
+    url: reqUrl.href,
+    body: recoupRequest,
+    codec: codecForRecoupConfirmation(),
+    http: ws.http,
+  });
 
   if (recoupConfirmation.old_coin_pub != cs.oldCoinPub) {
     throw Error(`Coin's oldCoinPub doesn't match reserve on recoup`);
diff --git a/src/operations/withdraw.ts b/src/operations/withdraw.ts
index 19b470e8..98969d21 100644
--- a/src/operations/withdraw.ts
+++ b/src/operations/withdraw.ts
@@ -46,7 +46,7 @@ import { updateExchangeFromUrl, getExchangeTrust } from 
"./exchanges";
 import { WALLET_EXCHANGE_PROTOCOL_VERSION } from "./versions";
 
 import * as LibtoolVersion from "../util/libtoolVersion";
-import { guardOperationException, scrutinizeTalerJsonResponse } from 
"./errors";
+import { guardOperationException } from "./errors";
 import { NotificationType } from "../types/notifications";
 import {
   getTimestampNow,
@@ -54,6 +54,7 @@ import {
   timestampCmp,
   timestampSubtractDuraction,
 } from "../util/time";
+import { httpPostTalerJson } from "../util/http";
 
 const logger = new Logger("withdraw.ts");
 
@@ -308,8 +309,13 @@ async function processPlanchet(
     `reserves/${planchet.reservePub}/withdraw`,
     exchange.baseUrl,
   ).href;
-  const resp = await ws.http.postJson(reqUrl, wd);
-  const r = await scrutinizeTalerJsonResponse(resp, 
codecForWithdrawResponse());
+
+  const r = await httpPostTalerJson({
+    url: reqUrl,
+    body: wd,
+    codec: codecForWithdrawResponse(),
+    http: ws.http,
+  });
 
   logger.trace(`got response for /withdraw`);
 
diff --git a/src/types/notifications.ts b/src/types/notifications.ts
index 644dfdc8..ac30b6fe 100644
--- a/src/types/notifications.ts
+++ b/src/types/notifications.ts
@@ -168,6 +168,7 @@ export interface PayOperationErrorNotification {
 
 export interface ProposalOperationErrorNotification {
   type: NotificationType.ProposalOperationError;
+  error: OperationError;
 }
 
 export interface TipOperationErrorNotification {
diff --git a/src/util/http.ts b/src/util/http.ts
index 0ac989a1..bc054096 100644
--- a/src/util/http.ts
+++ b/src/util/http.ts
@@ -14,6 +14,9 @@
  TALER; see the file COPYING.  If not, see <http://www.gnu.org/licenses/>
  */
 
+import { Codec } from "./codec";
+import { OperationFailedError } from "../operations/errors";
+
 /**
  * Helpers for doing XMLHttpRequest-s that are based on ES6 promises.
  * Allows for easy mocking for test cases.
@@ -172,3 +175,115 @@ export class BrowserHttpLib implements HttpRequestLibrary 
{
     // Nothing to do
   }
 }
+
+export interface PostJsonRequest<RespType> {
+  http: HttpRequestLibrary;
+  url: string;
+  body: any;
+  codec: Codec<RespType>;
+}
+
+/**
+ * Helper for making Taler-style HTTP POST requests with a JSON payload and 
response.
+ */
+export async function httpPostTalerJson<RespType>(
+  req: PostJsonRequest<RespType>,
+): Promise<RespType> {
+  const resp = await req.http.postJson(req.url, req.body);
+
+  if (resp.status !== 200) {
+    let exc: OperationFailedError | undefined = undefined;
+    try {
+      const errorJson = await resp.json();
+      const m = `received error response (status ${resp.status})`;
+      exc = new OperationFailedError({
+        type: "protocol",
+        message: m,
+        details: {
+          httpStatusCode: resp.status,
+          errorResponse: errorJson,
+        },
+      });
+    } catch (e) {
+      const m = "could not parse response JSON";
+      exc = new OperationFailedError({
+        type: "network",
+        message: m,
+        details: {
+          status: resp.status,
+        },
+      });
+    }
+    throw exc;
+  }
+  let json: any;
+  try {
+    json = await resp.json();
+  } catch (e) {
+    const m = "could not parse response JSON";
+    throw new OperationFailedError({
+      type: "network",
+      message: m,
+      details: {
+        status: resp.status,
+      },
+    });
+  }
+  return req.codec.decode(json);
+}
+
+
+export interface GetJsonRequest<RespType> {
+  http: HttpRequestLibrary;
+  url: string;
+  codec: Codec<RespType>;
+}
+
+/**
+ * Helper for making Taler-style HTTP GET requests with a JSON payload.
+ */
+export async function httpGetTalerJson<RespType>(
+  req: GetJsonRequest<RespType>,
+): Promise<RespType> {
+  const resp = await req.http.get(req.url);
+
+  if (resp.status !== 200) {
+    let exc: OperationFailedError | undefined = undefined;
+    try {
+      const errorJson = await resp.json();
+      const m = `received error response (status ${resp.status})`;
+      exc = new OperationFailedError({
+        type: "protocol",
+        message: m,
+        details: {
+          httpStatusCode: resp.status,
+          errorResponse: errorJson,
+        },
+      });
+    } catch (e) {
+      const m = "could not parse response JSON";
+      exc = new OperationFailedError({
+        type: "network",
+        message: m,
+        details: {
+          status: resp.status,
+        },
+      });
+    }
+    throw exc;
+  }
+  let json: any;
+  try {
+    json = await resp.json();
+  } catch (e) {
+    const m = "could not parse response JSON";
+    throw new OperationFailedError({
+      type: "network",
+      message: m,
+      details: {
+        status: resp.status,
+      },
+    });
+  }
+  return req.codec.decode(json);
+}
diff --git a/src/util/taleruri.ts b/src/util/taleruri.ts
index 2eaea284..30209d48 100644
--- a/src/util/taleruri.ts
+++ b/src/util/taleruri.ts
@@ -97,15 +97,6 @@ export function classifyTalerUri(s: string): TalerUriType {
   return TalerUriType.Unknown;
 }
 
-export function getOrderDownloadUrl(
-  merchantBaseUrl: string,
-  orderId: string,
-): string {
-  const u = new URL("proposal", merchantBaseUrl);
-  u.searchParams.set("order_id", orderId);
-  return u.href;
-}
-
 export function parsePayUri(s: string): PayUriResult | undefined {
   const pfx = "taler://pay/";
   if (!s.toLowerCase().startsWith(pfx)) {
@@ -133,7 +124,7 @@ export function parsePayUri(s: string): PayUriResult | 
undefined {
   }
 
   if (maybePath === "-") {
-    maybePath = "public/";
+    maybePath = "";
   } else {
     maybePath = decodeURIComponent(maybePath) + "/";
   }

-- 
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.