From 742312b85aa64dcb209d2826eba1143244968dd4 Mon Sep 17 00:00:00 2001 From: Dan Newton Date: Tue, 18 Aug 2020 12:05:05 +0100 Subject: [PATCH] NOTICK Do not replace stacktrace for local errors (#6635) We should not overwrite the stack trace of local errors thrown by `FlowContinuation.Throw` as it hides the real cause of the error. Exceptions received from peer nodes are still overwritten. --- .../statemachine/FlowStateMachineImpl.kt | 6 ++--- .../transitions/StartedFlowTransition.kt | 24 +++++++++---------- .../statemachine/FlowFrameworkTests.kt | 3 ++- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowStateMachineImpl.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowStateMachineImpl.kt index 7d2ac7f62a..267ff59aa6 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowStateMachineImpl.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowStateMachineImpl.kt @@ -237,12 +237,11 @@ class FlowStateMachineImpl(override val id: StateMachineRunId, } private fun Throwable.fillInLocalStackTrace(): Throwable { - fillInStackTrace() - // provide useful information that can be displayed to the user - // reflection use to access private field + // Fill in the stacktrace when the exception originates from another node when (this) { is UnexpectedFlowEndException -> { DeclaredField(UnexpectedFlowEndException::class.java, "peer", this).value?.let { + fillInStackTrace() stackTrace = arrayOf( StackTraceElement( "Received unexpected counter-flow exception from peer ${it.name}", @@ -255,6 +254,7 @@ class FlowStateMachineImpl(override val id: StateMachineRunId, } is FlowException -> { DeclaredField(FlowException::class.java, "peer", this).value?.let { + fillInStackTrace() stackTrace = arrayOf( StackTraceElement( "Received counter-flow exception from peer ${it.name}", diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/transitions/StartedFlowTransition.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/transitions/StartedFlowTransition.kt index 5ac73c799e..8190a2c49a 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/transitions/StartedFlowTransition.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/transitions/StartedFlowTransition.kt @@ -389,22 +389,20 @@ class StartedFlowTransition( } private fun convertErrorMessageToException(errorMessage: ErrorSessionMessage, peer: Party): Throwable { - val exception: Throwable = if (errorMessage.flowException == null) { - UnexpectedFlowEndException("Counter-flow errored", cause = null, originalErrorId = errorMessage.errorId) - } else { - errorMessage.flowException.originalErrorId = errorMessage.errorId - errorMessage.flowException - } - when (exception) { - // reflection used to access private field - is UnexpectedFlowEndException -> DeclaredField( + return if (errorMessage.flowException == null) { + UnexpectedFlowEndException("Counter-flow errored", cause = null, originalErrorId = errorMessage.errorId).apply { + DeclaredField( UnexpectedFlowEndException::class.java, "peer", - exception - ).value = peer - is FlowException -> DeclaredField(FlowException::class.java, "peer", exception).value = peer + this + ).value = peer + } + } else { + errorMessage.flowException.apply { + originalErrorId = errorMessage.errorId + DeclaredField(FlowException::class.java, "peer", errorMessage.flowException).value = peer + } } - return exception } private fun collectUncloseableSessions(sessionIds: Collection, checkpoint: Checkpoint): List { diff --git a/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt b/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt index 2a68ef2685..13e1eca04b 100644 --- a/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt +++ b/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt @@ -307,7 +307,7 @@ class FlowFrameworkTests { .isThrownBy { receivingFiber.resultFuture.getOrThrow() } .withMessage("Nothing useful") .withStackTraceContaining(ReceiveFlow::class.java.name) // Make sure the stack trace is that of the receiving flow - .withStackTraceContaining("Received counter-flow exception from peer") + .withStackTraceContaining("Received counter-flow exception from peer ${bob.name}") bobNode.database.transaction { assertThat(bobNode.internals.checkpointStorage.checkpoints()).isEmpty() } @@ -630,6 +630,7 @@ class FlowFrameworkTests { Notification.createOnNext(ReceiveFlow.START_STEP), Notification.createOnError(receiveFlowException) ) + assertThat(receiveFlowException).hasStackTraceContaining("Received unexpected counter-flow exception from peer ${bob.name}") assertSessionTransfers( aliceNode sent sessionInit(ReceiveFlow::class) to bobNode,