Wrap entire Client.getPayload() into try catch to avoid exceptions leaking (#2767)

* Wrap entire Client.getPayload() into try catch to avoid exceptions leaking

* Use akka's Unmarshal to get response payload

* Catch all non fatal exceptions in BitcoindInstance unit test
This commit is contained in:
Chris Stewart 2021-03-07 08:40:26 -06:00 committed by GitHub
parent 5a2f95c38e
commit 355fc6eefc
2 changed files with 17 additions and 29 deletions

View file

@ -1,9 +1,5 @@
package org.bitcoins.rpc
import java.io.{File, PrintWriter}
import java.net.URI
import java.nio.file.{Files, Path}
import akka.stream.StreamTcpException
import org.bitcoins.core.config.RegTest
import org.bitcoins.core.currency.Bitcoins
import org.bitcoins.rpc.client.common.BitcoindRpcClient
@ -18,8 +14,12 @@ import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil.newestBitcoindBinary
import org.bitcoins.testkit.util.{BitcoindRpcTest, FileUtil}
import org.scalatest.compatible.Assertion
import java.io.{File, PrintWriter}
import java.net.URI
import java.nio.file.{Files, Path}
import scala.concurrent.Future
import scala.io.Source
import scala.util.control.NonFatal
class BitcoindInstanceTest extends BitcoindRpcTest {
@ -144,7 +144,7 @@ class BitcoindInstanceTest extends BitcoindRpcTest {
logger.error(s"Got unexpected balance: $balance")
fail("Was able to connect to bitcoind after shutting down")
}
.recover { case _: StreamTcpException =>
.recover { case NonFatal(_) =>
()
}
} yield assert(balance > Bitcoins(0))

View file

@ -6,8 +6,8 @@ import akka.actor.ActorSystem
import akka.http.javadsl.model.headers.HttpCredentials
import akka.http.scaladsl.{Http, HttpExt}
import akka.http.scaladsl.model._
import akka.http.scaladsl.unmarshalling.Unmarshal
import akka.stream.StreamTcpException
import akka.util.ByteString
import com.fasterxml.jackson.core.JsonParseException
import org.bitcoins.asyncutil.AsyncUtil
import org.bitcoins.commons.jsonmodels.bitcoind.RpcOpts
@ -33,7 +33,8 @@ import play.api.libs.json._
import scala.concurrent._
import scala.concurrent.duration.DurationInt
import scala.sys.process._
import scala.util.{Failure, Success, Try}
import scala.util.control.NonFatal
import scala.util.{Failure, Success}
/** This is the base trait for Bitcoin Core
* RPC clients. It defines no RPC calls
@ -189,7 +190,7 @@ trait Client extends BitcoinSLogger with StartStopAsync[BitcoindRpcClient] {
val responseF = sendRequest(request)
val payloadF: Future[JsValue] =
responseF.flatMap(getPayload(_, command = "ping", request = request))
responseF.flatMap(getPayload(_))
// Ping successful if no error can be parsed from the payload
val parsedF = payloadF.map { payload =>
@ -255,7 +256,7 @@ trait Client extends BitcoinSLogger with StartStopAsync[BitcoindRpcClient] {
val responseF = sendRequest(request)
val payloadF: Future[JsValue] =
responseF.flatMap(getPayload(_, command, request, parameters))
responseF.flatMap(getPayload(_))
payloadF.map { payload =>
/** These lines are handy if you want to inspect what's being sent to and
@ -313,27 +314,14 @@ trait Client extends BitcoinSLogger with StartStopAsync[BitcoindRpcClient] {
* The command, parameters and request are given as debug parameters,
* and only used for printing diagnostics if things go belly-up.
*/
protected def getPayload(
response: HttpResponse,
command: String,
request: HttpRequest,
parameters: List[JsValue] = List.empty): Future[JsValue] = {
val payloadF = response.entity.dataBytes.runFold(ByteString.empty)(_ ++ _)
payloadF.flatMap { payload =>
Try(Json.parse(payload.decodeString(ByteString.UTF_8))) match {
case Failure(err) =>
if (network != MainNet) {
logger.error(s"Error when parsing result of command: $command")
logger.error(s"Parameters: ${Json.stringify(JsArray(parameters))}")
logger.error(s"Sent HTTP request: $request")
logger.error(s"Received HTTP response: $response")
logger.error(s"Error: $err")
}
Future.failed(err)
case Success(js) => Future.successful(js)
protected def getPayload(response: HttpResponse): Future[JsValue] = {
try {
Unmarshal(response).to[String].map { data =>
Json.parse(data)
}
} catch {
case NonFatal(exn) =>
Future.failed(exn)
}
}