A question for the Announcements porters

53 views
Skip to first unread message

jtu...@objektfabrik.de

unread,
Apr 27, 2012, 9:55:42 AM4/27/12
to va-sma...@googlegroups.com
Hi Marten and Sebastian,

I think you two were working on porting Announcements to VAST...

Why did you chose to implement AnnouncementSubscription>>deliver: like this: 

" deliver an announcement to receiver. 
Difference to Squeak implementation:
In case of failure, nothing is done
"

^ (self handles: anAnnouncement class ) 
ifTrue: [
[action cull: anAnnouncement cull: announcer] 
when: ExError
do: [ :sig | 
"still do not know what to do ..."
sig exitWith: nil  
]
]


I think this is wrong, but I am not sure if I am missing any detail that would make me agree with you.

The original implementation in Squeak, however, does at least not swallow the exception:

deliver: anAnnouncement
" deliver an announcement to receiver. In case of failure, it will be handled in separate process"

^ (self handles: anAnnouncement class ) ifTrue: [
[action cull: anAnnouncement cull: announcer] 
on: UnhandledError fork: [:ex | ex pass ]]

on:fork: is not available on VAST, and I am not sure if it adds any value in this case.

Why do I think it's wrong? Because whatever exception gets thrown and is uncaught in the actions that follow the Announcement (some perform: aMethod), it will be swallowed here.
For this it means that an unimplemented method was never signalled to the developer / user.

I see three options that seem to be better to me:

a) don't catch any errors here
b) pass the exception (which is equivalent to a, but would resemble the original behavior pretty closely)
c) repackage an exception into an AnnouncementProcessingFailed (or such) exception (not sure this adds any value, but you could configure Announcements to still ignore such exceptions if you need to).

But I think eating the exception is bad in this case.

So do you remember what your initial idea was for this change?

Joachim

Marten Feldtmann

unread,
Apr 28, 2012, 4:17:40 AM4/28/12
to va-sma...@googlegroups.com
No, it is simply wrong - and actually Sebastian put a "self halt" in the original ported code ... to make this more visible. Suggestions: patch it and upload it !

jtu...@objektfabrik.de

unread,
May 2, 2012, 3:36:39 AM5/2/12
to va-sma...@googlegroups.com
Hi Marten,

so I'll probably just remove the catching of exceptions completely and publish a new version to VASTGoodies. It may take another day or two, I pretty much was offline last weekend, enjoying spring and free time ;-)

The original code just spawns a new thread for pass:ing the exception, and I don't know (nor have time to test) what would happen in VAST in a GUI application if we did so (would a debugger contain any useful info anyways?), not handling exceptions is sure coming closest to it and works exactly the way I'd expect it to.

Thanks for your time!

Joachim

jtu...@objektfabrik.de

unread,
May 2, 2012, 3:01:26 PM5/2/12
to va-sma...@googlegroups.com
I uploaded a new version of Announcements to VASTGoodies.com. 
It doesn't catch exceptions in AnnouncmentSubscription>>#deliver: any more. 

For some users, this may mean their code looks broken now. Well, it already was, but they never knew... ;-)

Joachim
Reply all
Reply to author
Forward
0 new messages