From dd2efc3d78f2dfda44f8182f9638723dcb839781 Mon Sep 17 00:00:00 2001 From: Florian Dold Date: Mon, 20 Jul 2020 17:46:49 +0530 Subject: [PATCH] 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 | 41 +++++++------ 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, 190 insertions(+), 109 deletions(-) diff --git a/integrationtests/common.sh b/integrationtests/common.sh index d228d1ea5..d3f347672 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 { - 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 { - 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 { - 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 c8885c9e7..9def02b0e 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( - resp: HttpResponse, - codec: Codec, -): Promise { - // 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 a16190743..abcb2ad1d 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; - } + const orderClaimUrl = new URL( + `orders/${proposal.orderId}/claim`, + proposal.merchantBaseUrl, + ).href; + logger.trace("downloading contract from '" + orderClaimUrl + "'"); - if (resp.status !== 200) { - throw Error(`contract download failed with status ${resp.status}`); - } + + 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 e1c2325d7..d1b3c3bda 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 19b470e83..98969d213 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 644dfdc80..ac30b6fe2 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 0ac989a17..bc054096a 100644 --- a/src/util/http.ts +++ b/src/util/http.ts @@ -14,6 +14,9 @@ TALER; see the file COPYING. If not, see */ +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 { + http: HttpRequestLibrary; + url: string; + body: any; + codec: Codec; +} + +/** + * Helper for making Taler-style HTTP POST requests with a JSON payload and response. + */ +export async function httpPostTalerJson( + req: PostJsonRequest, +): Promise { + 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 { + http: HttpRequestLibrary; + url: string; + codec: Codec; +} + +/** + * Helper for making Taler-style HTTP GET requests with a JSON payload. + */ +export async function httpGetTalerJson( + req: GetJsonRequest, +): Promise { + 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 2eaea2846..30209d48a 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) + "/"; }