[DBCP] org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll() does not close all

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[DBCP] org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll() does not close all

garydgregory
org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll()
does not close all if one of delegated close() calls throws an exception.

I would think we would want to close all no matter what and then save the
first exception caught and rethrow it after all closes have been attempted.

Thoughts?

Gary
Reply | Threaded
Open this post in threaded view
|

Re: [DBCP] org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll() does not close all

Remko Popma-2
Good question.
I’ve never liked `close()` methods that throw exceptions. What is the client code supposed to do?

It certainly makes sense to me to first fulfill the promise of “closeAll”, which is to call `close` on all values (and not stop half-way through).

The remaining question is what to do with any exceptions (there could be multiple) that occurred during this process.

One idea is to rethrow the first one. Another is to throw a MultipleExceptions from which the client code can obtain all exceptions that occurred. But then again, given that there is little that the client code can do with these exceptions, why not _return_ a list of the exceptions that occurred?

Remko

(Shameless plug) Every java main() method deserves http://picocli.info

> On Jun 11, 2018, at 1:14, Gary Gregory <[hidden email]> wrote:
>
> org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll()
> does not close all if one of delegated close() calls throws an exception.
>
> I would think we would want to close all no matter what and then save the
> first exception caught and rethrow it after all closes have been attempted.
>
> Thoughts?
>
> Gary

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [DBCP] org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll() does not close all

Romain Manni-Bucau
Throw the first one and add others as being suppressed?

+1 to respect the naming and log any error (or store errors and let the
caller do it if logging is an issue, something like db.closeAll(); if
(db.hasError()) db.getErrors().forEach(log::error))


Le lun. 11 juin 2018 01:25, Remko Popma <[hidden email]> a écrit :

> Good question.
> I’ve never liked `close()` methods that throw exceptions. What is the
> client code supposed to do?
>
> It certainly makes sense to me to first fulfill the promise of “closeAll”,
> which is to call `close` on all values (and not stop half-way through).
>
> The remaining question is what to do with any exceptions (there could be
> multiple) that occurred during this process.
>
> One idea is to rethrow the first one. Another is to throw a
> MultipleExceptions from which the client code can obtain all exceptions
> that occurred. But then again, given that there is little that the client
> code can do with these exceptions, why not _return_ a list of the
> exceptions that occurred?
>
> Remko
>
> (Shameless plug) Every java main() method deserves http://picocli.info
>
> > On Jun 11, 2018, at 1:14, Gary Gregory <[hidden email]> wrote:
> >
> >
> org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll()
> > does not close all if one of delegated close() calls throws an exception.
> >
> > I would think we would want to close all no matter what and then save the
> > first exception caught and rethrow it after all closes have been
> attempted.
> >
> > Thoughts?
> >
> > Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DBCP] org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll() does not close all

Mark Thomas
In reply to this post by garydgregory
On 10/06/18 17:14, Gary Gregory wrote:
> org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll()
> does not close all if one of delegated close() calls throws an exception.
>
> I would think we would want to close all no matter what and then save the
> first exception caught and rethrow it after all closes have been attempted.
>
> Thoughts?

Agreed.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [DBCP] org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll() does not close all

garydgregory
Tracking here: https://issues.apache.org/jira/browse/DBCP-503

On Mon, Jun 11, 2018 at 2:27 AM Mark Thomas <[hidden email]> wrote:

> On 10/06/18 17:14, Gary Gregory wrote:
> >
> org.apache.commons.dbcp2.datasources.InstanceKeyDataSourceFactory.closeAll()
> > does not close all if one of delegated close() calls throws an exception.
> >
> > I would think we would want to close all no matter what and then save the
> > first exception caught and rethrow it after all closes have been
> attempted.
> >
> > Thoughts?
>
> Agreed.
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>