From b5b8f96cc94e3a3c0ee7d989819197ab5df393cd Mon Sep 17 00:00:00 2001 From: Florian Dold Date: Thu, 12 Mar 2020 19:25:38 +0530 Subject: improved error reporting / towards a working recoup --- src/operations/errors.ts | 104 +++++++++++++++++++++++++++++++++++++------- src/operations/exchanges.ts | 56 ++++++++++++++---------- src/operations/history.ts | 1 - src/operations/pending.ts | 2 + src/operations/recoup.ts | 34 ++++++++------- src/operations/refund.ts | 2 +- src/operations/reserves.ts | 45 ++++++++++++++++--- src/operations/withdraw.ts | 36 +++++++-------- 8 files changed, 199 insertions(+), 81 deletions(-) (limited to 'src/operations') diff --git a/src/operations/errors.ts b/src/operations/errors.ts index 7e97fdb3c..751a57111 100644 --- a/src/operations/errors.ts +++ b/src/operations/errors.ts @@ -1,8 +1,6 @@ -import { OperationError } from "../types/walletTypes"; - /* This file is part of GNU Taler - (C) 2019 GNUnet e.V. + (C) 2019-2020 Taler Systems SA GNU Taler is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -16,13 +14,26 @@ import { OperationError } from "../types/walletTypes"; GNU Taler; see the file COPYING. If not, see */ +/** + * Classes and helpers for error handling specific to wallet operations. + * + * @author Florian Dold + */ + +/** + * Imports. + */ +import { OperationError } from "../types/walletTypes"; +import { HttpResponse } from "../util/http"; +import { Codec } from "../util/codec"; + /** * This exception is there to let the caller know that an error happened, * but the error has already been reported by writing it to the database. */ export class OperationFailedAndReportedError extends Error { - constructor(message: string) { - super(message); + constructor(public operationError: OperationError) { + super(operationError.message); // Set the prototype explicitly. Object.setPrototypeOf(this, OperationFailedAndReportedError.prototype); @@ -34,14 +45,73 @@ export class OperationFailedAndReportedError extends Error { * responsible for recording the failure in the database. */ export class OperationFailedError extends Error { - constructor(message: string, public err: OperationError) { - super(message); + constructor(public operationError: OperationError) { + super(operationError.message); // Set the prototype explicitly. Object.setPrototypeOf(this, OperationFailedError.prototype); } } +/** + * 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. @@ -59,26 +129,28 @@ export async function guardOperationException( throw e; } if (e instanceof OperationFailedError) { - await onOpError(e.err); - throw new OperationFailedAndReportedError(e.message); + await onOpError(e.operationError); + throw new OperationFailedAndReportedError(e.operationError); } if (e instanceof Error) { console.log("guard: caught Error"); - await onOpError({ + const opErr = { type: "exception", message: e.message, details: {}, - }); - throw new OperationFailedAndReportedError(e.message); + } + await onOpError(opErr); + throw new OperationFailedAndReportedError(opErr); } console.log("guard: caught something else"); - await onOpError({ + const opErr = { type: "exception", message: "non-error exception thrown", details: { value: e.toString(), }, - }); - throw new OperationFailedAndReportedError(e.message); + }; + await onOpError(opErr); + throw new OperationFailedAndReportedError(opErr); } -} \ No newline at end of file +} diff --git a/src/operations/exchanges.ts b/src/operations/exchanges.ts index ed13a1e5b..04238e61d 100644 --- a/src/operations/exchanges.ts +++ b/src/operations/exchanges.ts @@ -115,72 +115,78 @@ async function updateExchangeWithKeys( keysResp = await r.json(); } catch (e) { const m = `Fetching keys failed: ${e.message}`; - await setExchangeError(ws, baseUrl, { + const opErr = { type: "network", details: { requestUrl: e.config?.url, }, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await setExchangeError(ws, baseUrl, opErr); + throw new OperationFailedAndReportedError(opErr); } let exchangeKeysJson: ExchangeKeysJson; try { exchangeKeysJson = codecForExchangeKeysJson().decode(keysResp); } catch (e) { const m = `Parsing /keys response failed: ${e.message}`; - await setExchangeError(ws, baseUrl, { + const opErr = { type: "protocol-violation", details: {}, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await setExchangeError(ws, baseUrl, opErr); + throw new OperationFailedAndReportedError(opErr); } const lastUpdateTimestamp = exchangeKeysJson.list_issue_date; if (!lastUpdateTimestamp) { const m = `Parsing /keys response failed: invalid list_issue_date.`; - await setExchangeError(ws, baseUrl, { + const opErr = { type: "protocol-violation", details: {}, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await setExchangeError(ws, baseUrl, opErr); + throw new OperationFailedAndReportedError(opErr); } if (exchangeKeysJson.denoms.length === 0) { const m = "exchange doesn't offer any denominations"; - await setExchangeError(ws, baseUrl, { + const opErr = { type: "protocol-violation", details: {}, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await setExchangeError(ws, baseUrl, opErr); + throw new OperationFailedAndReportedError(opErr); } const protocolVersion = exchangeKeysJson.version; if (!protocolVersion) { const m = "outdate exchange, no version in /keys response"; - await setExchangeError(ws, baseUrl, { + const opErr = { type: "protocol-violation", details: {}, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await setExchangeError(ws, baseUrl, opErr); + throw new OperationFailedAndReportedError(opErr); } const versionRes = compare(WALLET_EXCHANGE_PROTOCOL_VERSION, protocolVersion); if (versionRes?.compatible != true) { const m = "exchange protocol version not compatible with wallet"; - await setExchangeError(ws, baseUrl, { + const opErr = { type: "protocol-incompatible", details: { exchangeProtocolVersion: protocolVersion, walletProtocolVersion: WALLET_EXCHANGE_PROTOCOL_VERSION, }, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await setExchangeError(ws, baseUrl, opErr); + throw new OperationFailedAndReportedError(opErr); } const currency = Amounts.parseOrThrow(exchangeKeysJson.denoms[0].value) @@ -195,7 +201,7 @@ async function updateExchangeWithKeys( let recoupGroupId: string | undefined = undefined; await ws.db.runWithWriteTransaction( - [Stores.exchanges, Stores.denominations], + [Stores.exchanges, Stores.denominations, Stores.recoupGroups, Stores.coins], async tx => { const r = await tx.get(Stores.exchanges, baseUrl); if (!r) { @@ -231,10 +237,11 @@ async function updateExchangeWithKeys( // Handle recoup const recoupDenomList = exchangeKeysJson.recoup ?? []; const newlyRevokedCoinPubs: string[] = []; - for (const recoupDenomPubHash of recoupDenomList) { + console.log("recoup list from exchange", recoupDenomList); + for (const recoupInfo of recoupDenomList) { const oldDenom = await tx.getIndexed( Stores.denominations.denomPubHashIndex, - recoupDenomPubHash, + recoupInfo.h_denom_pub, ); if (!oldDenom) { // We never even knew about the revoked denomination, all good. @@ -243,18 +250,21 @@ async function updateExchangeWithKeys( if (oldDenom.isRevoked) { // We already marked the denomination as revoked, // this implies we revoked all coins + console.log("denom already revoked"); continue; } + console.log("revoking denom", recoupInfo.h_denom_pub); oldDenom.isRevoked = true; await tx.put(Stores.denominations, oldDenom); const affectedCoins = await tx - .iterIndexed(Stores.coins.denomPubIndex) + .iterIndexed(Stores.coins.denomPubHashIndex, recoupInfo.h_denom_pub) .toArray(); for (const ac of affectedCoins) { newlyRevokedCoinPubs.push(ac.coinPub); } } if (newlyRevokedCoinPubs.length != 0) { + console.log("recouping coins", newlyRevokedCoinPubs); await createRecoupGroup(ws, tx, newlyRevokedCoinPubs); } }, @@ -263,7 +273,7 @@ async function updateExchangeWithKeys( if (recoupGroupId) { // Asynchronously start recoup. This doesn't need to finish // for the exchange update to be considered finished. - processRecoupGroup(ws, recoupGroupId).catch((e) => { + processRecoupGroup(ws, recoupGroupId).catch(e => { console.log("error while recouping coins:", e); }); } diff --git a/src/operations/history.ts b/src/operations/history.ts index 2cf215a5a..c09aa8d30 100644 --- a/src/operations/history.ts +++ b/src/operations/history.ts @@ -41,7 +41,6 @@ import { } from "../types/history"; import { assertUnreachable } from "../util/assertUnreachable"; import { TransactionHandle, Store } from "../util/query"; -import { ReserveTransactionType } from "../types/ReserveTransaction"; import { timestampCmp } from "../util/time"; /** diff --git a/src/operations/pending.ts b/src/operations/pending.ts index 08ec3fc9e..a628d6130 100644 --- a/src/operations/pending.ts +++ b/src/operations/pending.ts @@ -427,6 +427,8 @@ async function gatherRecoupPending( type: PendingOperationType.Recoup, givesLifeness: true, recoupGroupId: rg.recoupGroupId, + retryInfo: rg.retryInfo, + lastError: rg.lastError, }); }); } diff --git a/src/operations/recoup.ts b/src/operations/recoup.ts index 842a67b87..3097dd05c 100644 --- a/src/operations/recoup.ts +++ b/src/operations/recoup.ts @@ -40,7 +40,7 @@ import { import { codecForRecoupConfirmation } from "../types/talerTypes"; import { NotificationType } from "../types/notifications"; -import { processReserve } from "./reserves"; +import { forceQueryReserve } from "./reserves"; import * as Amounts from "../util/amounts"; import { createRefreshGroup, processRefreshGroup } from "./refresh"; @@ -48,7 +48,7 @@ import { RefreshReason, OperationError } from "../types/walletTypes"; import { TransactionHandle } from "../util/query"; import { encodeCrock, getRandomBytes } from "../crypto/talerCrypto"; import { getTimestampNow } from "../util/time"; -import { guardOperationException } from "./errors"; +import { guardOperationException, scrutinizeTalerJsonResponse } from "./errors"; async function incrementRecoupRetry( ws: InternalWalletState, @@ -133,17 +133,17 @@ 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); - if (resp.status !== 200) { - throw Error("recoup request failed"); - } - const recoupConfirmation = codecForRecoupConfirmation().decode( - await resp.json(), + const recoupConfirmation = await scrutinizeTalerJsonResponse( + resp, + codecForRecoupConfirmation(), ); if (recoupConfirmation.reserve_pub !== reservePub) { throw Error(`Coin's reserve doesn't match reserve on recoup`); } + // FIXME: verify signature + // FIXME: verify that our expectations about the amount match await ws.db.runWithWriteTransaction( @@ -178,8 +178,8 @@ async function recoupWithdrawCoin( type: NotificationType.RecoupFinished, }); - processReserve(ws, reserve.reservePub).catch(e => { - console.log("processing reserve after recoup failed:", e); + forceQueryReserve(ws, reserve.reservePub).catch(e => { + console.log("re-querying reserve after recoup failed:", e); }); } @@ -196,12 +196,11 @@ 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); - if (resp.status !== 200) { - throw Error("recoup request failed"); - } - const recoupConfirmation = codecForRecoupConfirmation().decode( - await resp.json(), + const recoupConfirmation = await scrutinizeTalerJsonResponse( + resp, + codecForRecoupConfirmation(), ); if (recoupConfirmation.old_coin_pub != cs.oldCoinPub) { @@ -283,11 +282,14 @@ async function processRecoupGroupImpl( if (forceNow) { await resetRecoupGroupRetry(ws, recoupGroupId); } + console.log("in processRecoupGroupImpl"); const recoupGroup = await ws.db.get(Stores.recoupGroups, recoupGroupId); if (!recoupGroup) { return; } + console.log(recoupGroup); if (recoupGroup.timestampFinished) { + console.log("recoup group finished"); return; } const ps = recoupGroup.coinPubs.map((x, i) => @@ -317,11 +319,11 @@ export async function createRecoupGroup( const coinPub = coinPubs[coinIdx]; const coin = await tx.get(Stores.coins, coinPub); if (!coin) { - recoupGroup.recoupFinishedPerCoin[coinIdx] = true; + await putGroupAsFinished(tx, recoupGroup, coinIdx); continue; } if (Amounts.isZero(coin.currentAmount)) { - recoupGroup.recoupFinishedPerCoin[coinIdx] = true; + await putGroupAsFinished(tx, recoupGroup, coinIdx); continue; } coin.currentAmount = Amounts.getZero(coin.currentAmount.currency); diff --git a/src/operations/refund.ts b/src/operations/refund.ts index 9d1c5308e..c856bb7d2 100644 --- a/src/operations/refund.ts +++ b/src/operations/refund.ts @@ -440,7 +440,7 @@ async function processPurchaseApplyRefundImpl( body = await resp.json(); } catch {} const m = "refund request (at exchange) failed"; - throw new OperationFailedError(m, { + throw new OperationFailedError({ message: m, type: "network", details: { diff --git a/src/operations/reserves.ts b/src/operations/reserves.ts index c909555fe..efca08a45 100644 --- a/src/operations/reserves.ts +++ b/src/operations/reserves.ts @@ -202,6 +202,35 @@ export async function createReserve( return resp; } +/** + * Re-query the status of a reserve. + */ +export async function forceQueryReserve( + ws: InternalWalletState, + reservePub: string, +): Promise { + await ws.db.runWithWriteTransaction([Stores.reserves], async (tx) => { + const reserve = await tx.get(Stores.reserves, reservePub); + if (!reserve) { + return; + } + // Only force status query where it makes sense + switch (reserve.reserveStatus) { + case ReserveRecordStatus.DORMANT: + case ReserveRecordStatus.WITHDRAWING: + case ReserveRecordStatus.QUERYING_STATUS: + break; + default: + return; + } + reserve.reserveStatus = ReserveRecordStatus.QUERYING_STATUS; + reserve.retryInfo = initRetryInfo(); + await tx.put(Stores.reserves, reserve); + + }); + await processReserve(ws, reservePub); +} + /** * First fetch information requred to withdraw from the reserve, * then deplete the reserve, withdrawing coins until it is empty. @@ -408,7 +437,7 @@ async function updateReserve( console.log("got reserves/${RESERVE_PUB} response", await resp.json()); if (resp.status === 404) { const m = "reserve not known to the exchange yet" - throw new OperationFailedError(m, { + throw new OperationFailedError({ type: "waiting", message: m, details: {}, @@ -420,12 +449,13 @@ async function updateReserve( } catch (e) { logger.trace("caught exception for reserve/status"); const m = e.message; - await incrementReserveRetry(ws, reservePub, { + const opErr = { type: "network", details: {}, message: m, - }); - throw new OperationFailedAndReportedError(m); + }; + await incrementReserveRetry(ws, reservePub, opErr); + throw new OperationFailedAndReportedError(opErr); } const respJson = await resp.json(); const reserveInfo = codecForReserveStatus().decode(respJson); @@ -600,13 +630,14 @@ async function depleteReserve( logger.trace(`got denom list`); if (denomsForWithdraw.length === 0) { const m = `Unable to withdraw from reserve, no denominations are available to withdraw.`; - await incrementReserveRetry(ws, reserve.reservePub, { + const opErr = { type: "internal", message: m, details: {}, - }); + }; + await incrementReserveRetry(ws, reserve.reservePub, opErr); console.log(m); - throw new OperationFailedAndReportedError(m); + throw new OperationFailedAndReportedError(opErr); } logger.trace("selected denominations"); diff --git a/src/operations/withdraw.ts b/src/operations/withdraw.ts index 478aa4ceb..09d912bcc 100644 --- a/src/operations/withdraw.ts +++ b/src/operations/withdraw.ts @@ -1,6 +1,6 @@ /* This file is part of GNU Taler - (C) 2019 GNUnet e.V. + (C) 2019-2029 Taler Systems SA GNU Taler is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -33,7 +33,10 @@ import { WithdrawDetails, OperationError, } from "../types/walletTypes"; -import { WithdrawOperationStatusResponse, codecForWithdrawOperationStatusResponse } from "../types/talerTypes"; +import { + codecForWithdrawOperationStatusResponse, + codecForWithdrawResponse, +} from "../types/talerTypes"; import { InternalWalletState } from "./state"; import { parseWithdrawUri } from "../util/taleruri"; import { Logger } from "../util/logging"; @@ -41,7 +44,7 @@ import { updateExchangeFromUrl, getExchangeTrust } from "./exchanges"; import { WALLET_EXCHANGE_PROTOCOL_VERSION } from "./versions"; import * as LibtoolVersion from "../util/libtoolVersion"; -import { guardOperationException } from "./errors"; +import { guardOperationException, scrutinizeTalerJsonResponse } from "./errors"; import { NotificationType } from "../types/notifications"; import { getTimestampNow, @@ -49,7 +52,6 @@ import { timestampCmp, timestampSubtractDuraction, } from "../util/time"; -import { Store } from "../util/query"; const logger = new Logger("withdraw.ts"); @@ -62,7 +64,7 @@ function isWithdrawableDenom(d: DenominationRecord) { ); const remaining = getDurationRemaining(lastPossibleWithdraw, now); const stillOkay = remaining.d_ms !== 0; - return started && stillOkay; + return started && stillOkay && !d.isRevoked; } /** @@ -144,8 +146,9 @@ async function getPossibleDenoms( .iterIndex(Stores.denominations.exchangeBaseUrlIndex, exchangeBaseUrl) .filter(d => { return ( - d.status === DenominationStatus.Unverified || - d.status === DenominationStatus.VerifiedGood + (d.status === DenominationStatus.Unverified || + d.status === DenominationStatus.VerifiedGood) && + !d.isRevoked ); }); } @@ -199,13 +202,12 @@ async function processPlanchet( wd.reserve_pub = planchet.reservePub; wd.reserve_sig = planchet.withdrawSig; wd.coin_ev = planchet.coinEv; - const reqUrl = new URL(`reserves/${planchet.reservePub}/withdraw`, exchange.baseUrl).href; + const reqUrl = new URL( + `reserves/${planchet.reservePub}/withdraw`, + exchange.baseUrl, + ).href; const resp = await ws.http.postJson(reqUrl, wd); - if (resp.status !== 200) { - throw Error(`unexpected status ${resp.status} for withdraw`); - } - - const r = await resp.json(); + const r = await scrutinizeTalerJsonResponse(resp, codecForWithdrawResponse()); const denomSig = await ws.cryptoApi.rsaUnblind( r.ev_sig, @@ -236,8 +238,8 @@ async function processPlanchet( type: CoinSourceType.Withdraw, coinIndex: coinIdx, reservePub: planchet.reservePub, - withdrawSessionId: withdrawalSessionId - } + withdrawSessionId: withdrawalSessionId, + }, }; let withdrawSessionFinished = false; @@ -458,11 +460,11 @@ async function processWithdrawCoin( if (planchet) { const coin = await ws.db.get(Stores.coins, planchet.coinPub); - + if (coin) { console.log("coin already exists"); return; - } + } } if (!withdrawalSession.planchets[coinIndex]) { -- cgit v1.2.3