SwallowedException false positive when extension function is used

[MartinRajniak]

Context

I want to transform one exception into another before rethrowing.

For that I am using extension function like this:

    fun IOException.toMyException(): MyException = MyException(this)

Usage looks like this

    try {
        // ...
    } catch(e: IOException) {
        throw e.toMyException()
    }

But I am getting SwallowedException error for that one. Based on the documentation I assume this is not supported https://detekt.github.io/detekt/exceptions.html#compliant-code-4

Expected Behavior of the rule

I would expect this rule to ignore extensions that don't swallow the original exception.

[marschwar]

You are right. throw e.toMyException()should be considered equivalent to throw toMyException(e) but it is not. Thank you for reporting this. Would you like to look into the issue yourself and provide a pull request?

[MartinRajniak]

Yes, I would love to - I just need some hints.

Currently, I think it is flagged because of this use case

    try {
        // ...
    } catch(e: IOException) {
        throw MyException(e.message) // e is swallowed
    }

I don't have much experience here, so can't tell whether it is possible to differentiate between calling functions from a class vs. extension functions.

[marschwar]

You might be right. Since there really is no way of knowing if the extension function swallows the exception or not this may not be possible at all. The extension function could be:

    fun IOException.toMyException(): MyException = MyException(this)
   // or
    fun IOException.toMyException(): MyException = MyException(this.message)

Maybe someone else has an idea on how to address this.

[MartinRajniak]

Actually, I don't think that this is checked once an exception is passed to the function.

    try {
        // ...
    } catch(e: IOException) {
        throw MyException(e)
    }
    try {
        // ...
    } catch(e: IOException) {
        println(e) // logging is ok here
    }

both MyException constructor and println function can swallow the exception (I have tried swallowing it in the body and it works - no flagging)

I think the check is only for immediate handling and the condition might be:

• either exception is not used (passed into another function),
• or it is used but a function is invoked on the exception (to avoid .toString())

[marschwar]

Looking at the tests for the rule it looks to me that you either do something with the exception (without rethrowing) or you throw an exception using the exception as a parameter. There could be more to it though.

You could also "game the system" by doing

    try {
      // ...
    } catch (e: IOException) {
        e.throwMyException()
    }

private fun IOException.throwMyException(): Nothing = throw MyException(this)

Though I am not sure I really like this "solution".

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

[VitalyVPinchuk]

May I try to apply a simple fix for the issue? Or is it necessary to come up with a general solution (which I don't have obviously)?

[BraisGabin]

Go ahead!

[VitalyVPinchuk]

@BraisGabin Should it closed too?

[BraisGabin]

Closing. The reason is at #5046

[TWiStErRob]

I ran into this twice in a year now. My utility is

fun Throwable.withRootCause(cause: Throwable): Throwable {
	require(cause !in generateSequence(this) { it.cause })
	generateSequence(this) { it.cause }.last().initCause(cause)
	return this
}

it adds extra information to the throwable:

internal fun assertOutput(command: List<Any>, expected: String, message: String? = null) {
	try {
		val output = command.map(Any?::toString).runCommand()
		assertEquals(expected.normalize(), output.normalize(), message)
	} catch (@Suppress("SwallowedException") ex: Throwable) { // Detekt doesn't see into the extension fun.
		throw ex.withRootCause(IllegalArgumentException("Command: $command"))
	}
}

I see how complex "whitelisting" is. I think https://github.com/detekt/detekt/issues/5510 might be the solution, the problem is that it might need some framework changes, because the violation is reported on the catch variable, but on the withRootCause call. For this reason I think it might need special logic in the rule, OR the framework to consider additional locations in the suppression process.

Note: luckily I could rewrite this to .apply { setRootCause(...) } which looks ugly, but should make detekt pass, but as soon as I need to reconstruct the throwable (because stack trace manipulation or other magic, this falls apart, so not a general solution.