You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2018/06/10 16:14:45 UTC

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

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

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

Posted by Gary Gregory <ga...@gmail.com>.
Tracking here: https://issues.apache.org/jira/browse/DBCP-503

On Mon, Jun 11, 2018 at 2:27 AM Mark Thomas <ma...@apache.org> 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: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

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

Posted by Mark Thomas <ma...@apache.org>.
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


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

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <re...@gmail.com> 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 <ga...@gmail.com> 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: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

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

Posted by Remko Popma <re...@gmail.com>.
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 <ga...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org