scala-arm icon indicating copy to clipboard operation
scala-arm copied to clipboard

Missing exception handling for opening of the resource in AbstractManagedResource.acquireFo

Open naree opened this issue 8 years ago • 4 comments

Hello,

I noticed that AbstractManagedResource.acquireFor is missing exception handing for opening of the resource.

https://github.com/jsuereth/scala-arm/blob/master/src/main/scala/resource/AbstractManagedResource.scala#L87

If this is a bug I am happy to create a fix and create a pull request. But I just wanted to check if there was a reason for not handling exception for opening the resource at this point.

Cheers, Naree

naree avatar May 06 '16 09:05 naree

Thanks for the question!

Yeah, we are explicitly not handling that exception. If opening the resource fails, that should be handled by an outer-scope. Now, when nesting resources, the "outer" resource will catch exceptions from the "inner" resource and make sure it closes itself.

I could be persuaded to change it so we handle open-failures similarly to close failures, if you have a good argument.

jsuereth avatar May 11 '16 14:05 jsuereth

Hi Josh,

Thanks for the reply. Let me try to explain the problem I have with an example.

I like that scala-arm provides monadic operations. I can write code that creates a monadic composition which describes the effect.

  val statement = for {
    connection <- managed(DriverManager.getConnection(jdbcConfig.url, jdbcConfig.user, jdbcConfig.password))
    statement <- managed(connection.createStatement())
  } yield statement

and then execute it when ready without worrying about exception handling and closing resources.

  statement.acquireAndGet { statement => { statement.executeQuery(sql) } }

However, as scala-arm does not handle open failures, in reality the code becomes like this:

  try { 
    statement.acquireAndGet { statement => { statement.executeQuery(sql) } }
  } catch {
    case e: Exception => Left(DBExecutionFailure(e.toMessage())
  }

As you can see the exception handling is leaked outside scala-arm library and side-effect is no longer contained within the monadic composition. If there is a way to solve it without introducing open-failures handling, I will be happy to use it.

Thanks! Naree

naree avatar May 15 '16 19:05 naree

@naree No, it's more of a design decision on my part than anything else. I think I use the fact that open-throws-exception to shorten a couple coding paths. I'l treat this as a bug and try to fix for 2.0.

Sorry it took me a while to think through the issue. Can you give me a test case for the issue so I make sure it's resolved? I'll implement the back-end code.

jsuereth avatar May 23 '16 16:05 jsuereth

any progress here, guys?

andyglow avatar Oct 20 '21 18:10 andyglow