On 18 May 2014 17:11, GESCONSULTOR - Óscar Bou <[email protected]>wrote:

>
> I've been working changing the current implementation to a new one based
> on the EvenBus the whole weekend and it works wonderfully :-))
>
>
very coo.



> With current implementation I have had enough for all my use cases.
>
> There's just one observation related to Exception handling on event
> subscribers.
>
> By default, Guava's EventBus will hidden any exception thrown by an event
> subscriber. In last implementations (version 16) they allowed a method to
> create the EventBus with a SubscriberExceptionHandler [1].
>
> There's a discussion about its implementation on [2].
>
>
Hi Oscar,

My thoughts on this is that the subscribing service should be able to veto
the interaction by throwing different sorts of exception:
- a RecoverableException (or ApplicationException, its subclass) should
abort the transaction and render a user-friendly error
- a NonRecoverableException (or any other RuntimeException, really) should
abort the transaction and be treated as an unexpected/serious error.

Just playing around with this now... the code is something like:

    protected EventBus newEventBus() {
        return new EventBus(newEventBusSubscriberExceptionHandler());
    }

    protected SubscriberExceptionHandler
newEventBusSubscriberExceptionHandler() {
        return new SubscriberExceptionHandler(){
            @Override
            public void handleException(Throwable exception,
SubscriberExceptionContext context) {
                getTransactionManager().getTransaction().setAbortCause(new
IsisApplicationException(exception));
            }
        };
    }

I then use the ExceptionRecognizer infrastructure to do the render either
as non-fatal or fatal error.

Need to test how this plays with the integration tests, but is working ok
with the Wicket viewer.

Thoughts?

Dan





> If not present, in order to, at least, show your exception on the log, you
> must implement something like:
>
>     @Programmatic
>     @Subscribe
>     public void on(final AssetCollectionWithRelationshipAddedToEvent
> event) {
>         LOG.debug("Event Received: {}", event.toString());
>
>         // Event Handlers should not throw any exceptions. See:
>         //
> http://stackoverflow.com/questions/9233966/guava-eventbus-dont-catch-runtimeexception
>         try {
>             this.upsertRelationshipFromEvent(event.getSource(),
> event.getValue(), event.getIdentifier().getMemberName());
>         } catch (final Exception e) {
>             e.printStackTrace();
>         }
>
>     }
>
>
> Not sure about what would be the best implementation. I don't see sample
> implementations neither...
>
> Regards,
>
> Oscar
>
>
>
> [1]
> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#190
> [2] https://code.google.com/p/guava-libraries/issues/detail?id=766
>
>
>
>
> El 17/05/2014, a las 09:57, Dan Haywood <[email protected]>
> escribió:
>
>
> On 17 May 2014 07:42, GESCONSULTOR <[email protected]> wrote:
>
> Hi Dan.
>
> Yes, I'm receiving old messages also...
>
>
> thx for the confirmation on that.  Wil have to monitor things, see if
> improves.
>
>
>
> That's related to DN, not to the EventBus.
>
>
> OK, glad the EventBus thing is working fine.  Think it's a really important
> feature that you and I have implemented there, hoping to use it within
> Estatio before too long.
>
>
>
>
> The problem is not with the semantics of the operation, but with it
> "eating" a database exception and returning simply false instead of
> throwing it to be catched anywhere.
>
> The point is that at finish the user cannot be sure that the operation is
> retiring false only because the set contained the object.
>
> There's another case when it returns false: a database exception was
> thrown so the object has not been persisted to the database.
>
>
> I'm still not convinced that DN is wrong here.  Yes, an exception gets
> thrown and swallowed/converted to false, but I see that as an
> implementation detail; DN detects the uniqueness constraint and from that
> correctly concludes that the object is already present and therefore should
> return false.
>
> Perhaps we should look at this the other way around... what should the user
> see in different scenarios:
>
> Here's what I think is the current behaviour:
>
> given the reference is not in the collection
> when the reference is added
> then add() returns true
> and then user sees ...???
>
> given the reference is already in the collection
> when the reference is added
> then add() returns false
> and then user sees ...???
>
> given (the reference may or may not be in the collection)
> when an attempt is made to add the reference
> and when a system exception occurs (eg database server down)
> then add() should throw an exception.
> and then user sees ...???
>
> Dan
>
>
>
>
> HTH,
>
> Oscat
>
>
>
>
> Disculpa que sea breve.
> Enviado desde mi móvil
>
> El 16/05/2014, a las 16:39, Dan Haywood <[email protected]>
>
> escribió:
>
>
> On 16 May 2014 09:43, GESCONSULTOR - Óscar Bou <[email protected]
> wrote:
>
>
> Hi all.
>
> Hi Oscar,
> this has only just come through to my mailbox, even though you sent it ~5
> hours ago.  I've also been having a variety of commit messages etc
>
> delayed,
>
> some by as much as 5 days(!).  Not sure if the problem is at my end or at
> ASFs (but I suspect perhaps the latter...)
>
> Have you noticed any delays?
>
>
>
> I'm implementing the new support for automatic simple and collection
> properties change events (@PostsPropertyChangedEvent, @
> PostsCollectionAddedToEvent, @PostsCollectionRemovedFromEvent) and the
>
> new
>
> mechanism works really nice :-))
>
> I've just initially forgot to register the service as an event
>
> subscriber,
>
> thinking that it was unnecessary. So perhaps auto-registering the
>
> service
>
> when "detecting" the guava's @Subscribe annotation would be a good
> enhancement.
>
> OK, I'll raise a ticket.
>
>
>
>
> I've found that an exception thrown by DN has been hidden by Isis.
>
> Just so I'm clear ... this is unrelated to the event bus stuff, correct?
>
>
>
>
> The root-cause is because I have not properly defined the @Inheritance
> mappings correctly, but the relevant thing from Isis perspective is
>
> that DN
>
> does not throw any exception if, on a collection annotated with @Join,
>
> the
>
> "add" is not properly executed.
>
>
> When the
>
> this.getAggregatedServices().add(service);
>
>
> Is called, the following exception occurs:
> [snip]
> But DN does not throw any exception. It simply returns false when
>
> exiting
>
> the
>
> this.getAggregatedServices().add(service);
>
> method ...
>
> The DN implementation is on [1].
>
> So seems that it's mandatory to evaluate the result of "add" !!!!
>
> I don't think this is a bug, I believe that DN is conforming to the
> semantics of Set#add(...).  I've just mailed Andy @ DN for clarification.
>
>
>
> I understand that an ApplicationException should be raised in all cases.
> What would be a convenient "idiom" we can all use?
>
> From the Javadocs:
>
> Adds the specified element to this set if it is not already present
> (optional operation). More formally, adds the specified element e to this
> set if the set contains no element e2 such that (e==null ? e2==null :
> e.equals(e2)). If this set already contains the element, the call leaves
> the set unchanged and returns false.
>
> If it returns false, it means that the element is already in the set; the
> post-condition is the same.   So the idiom is simply to ignore the return
> code, it doesn't matter if true of false is returned.
>
>
> Of course, this relies on a correct implementation of equals(); one
>
> option
>
> is to use Isis' ObjectContracts helper class.
>
>
> HTH
> Dan
>
>
>
>
>
> Thanks,
>
> Oscar
>
>
>
>
>
>
> [1]
>
>
> https://github.com/datanucleus/datanucleus-core/blob/master/src/main/java/org/datanucleus/store/types/wrappers/backed/TreeSet.java#L719
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> Óscar Bou Bou
> Responsable de Producto
> Auditor Jefe de Certificación ISO 27001 en BSI
> CISA, CRISC, APMG ISO 20000, ITIL-F
>
>    902 900 231 / 620 267 520
>    http://www.twitter.com/oscarbou
>
>    http://es.linkedin.com/in/oscarbou
>
>    http://www.GesConsultor.com <http://www.gesconsultor.com/>
>
>
>
> Este mensaje y los ficheros anexos son confidenciales. Los mismos
> contienen información reservada que no puede ser difundida. Si usted ha
> recibido este correo por error, tenga la amabilidad de eliminarlo de su
> sistema y avisar al remitente mediante reenvío a su dirección electrónica;
> no deberá copiar el mensaje ni divulgar su contenido a ninguna persona.
> Su dirección de correo electrónico junto a sus datos personales constan en
> un fichero titularidad de Gesdatos Software, S.L. cuya finalidad es la de
> mantener el contacto con Ud. Si quiere saber de qué información disponemos
> de Ud., modificarla, y en su caso, cancelarla, puede hacerlo enviando un
> escrito al efecto, acompañado de una fotocopia de su D.N.I. a la siguiente
> dirección: Gesdatos Software, S.L. , Paseo de la Castellana, 153 bajo -
> 28046 (Madrid), y Avda. Cortes Valencianas num. 50, 1ºC - 46015 (Valencia).
> Asimismo, es su responsabilidad comprobar que este mensaje o sus archivos
> adjuntos no contengan virus informáticos, y en caso que los tuvieran
> eliminarlos.
>
>
>
>
>
>

Reply via email to