Hi Chris, hi all,

If I do this:

    [ Error signal. 'resumed' ] on: Error do: [ : err | err resume ]

I get an IllegalResumeAttempt exception, as it should be. Developers
or Squeak app users can always press Proceed (and hopefully they now
what they are doing), but you cannot "click proceed" in any regular
error handling code. So you cannot proceed non-resumable exceptions
in, for example, unattended batch jobs.

SmalltalkCI is such an unattended job, and I would like a test case to
fail in error because of ModificationForbidden. I neither want it to
halt the whole test suite (as it does now), nor to silently continue
as if there were no problem in the tested code.

Concerning the discussion how to sort it into the class hierarchy: we
should probably just think about which kinds of existing error
handlers should catch it. This makes it a question of compatibility
with existing code.
- on: Error do: cannot resume, but can be used to skip over the rest
of an operation and continue with the next work item. Errors are
caught by SUnit and smalltalkCI.
- on: Warning do: can resume, and I believe it will in most of the
cases, maybe with some logging involved. Warnings are not handled by
SUnit or smalltalkCI, but by SmalltalkImage>>#run:
- on: Notification do: can resume, but I am not sure why one would
write such a handler for all kinds of Notifications.
- on: Exception do: [:ex | ex isResumable ifTrue: [ex resume] ...] can
obviously resume, the intention is clear, but I don't think this is a
common idiom, is it?
- on: ModificationForbidden do: can resume and would be the ideal
choice to overcome our concerns, of course, but it is not found in
existing code bases yet.

I suppose inheriting from Error is "fail-safest", but not necessarily
fail-operational. Unattended services that somehow worked with the old
ways may break. If we don't want to break things (that are somehow
wrong according to contemporary notion), we could make it a Warning in
the Squeak 5.x release stream and turn it into an error with Squeak
6.0. That is: make it an Error today in Trunk, but if we would create
a 5.4 release, we would have to remember changing it back... :-/

Kind regards,
Jakob

Am Fr., 10. Apr. 2020 um 02:18 Uhr schrieb Chris Muller <ma.chris.m at gmail.com>:
>
> Hi Eliot,
>
> Right, ALL errors are mechanically resumable, regardless of #isResumable.  See?
>
>    [ Error signal. 'resumed' ] on: Error do: [ : err | err resume ]         "resumed"
>
> ... so we're just talking about proper semantics.  Given your statements, what differentiates for you, a Warning vs. a resumable Error.   To me, the latter seems like a misnomer...
>
> On Thu, Apr 9, 2020 at 6:21 PM Eliot Miranda <eliot.miranda at gmail.com> wrote:
>>
>> Hi Chris,
>>
>>
>> On Apr 9, 2020, at 3:16 PM, Chris Muller <asqueaker at gmail.com> wrote:
>>
>> 
>> ModificationForbidden is resumable like a Warning, and unlike most Errors.  Perhaps it should be a Warning.
>>
>>
>> One can override isResumable. There’s no invariant that’s a subclass of Error can’t be resumable.  IMO many more Error subclasses that aren’t should be isResumable.
>>
>>
>>
>> Proper signaling and handling are independent of each other.  Please evaluate your decision from the handling side too -- whether it'd be better for TestRunner's handling to include ModificationForbidden.
>>
>>  - Chris
>>
>> On Thu, Apr 9, 2020 at 12:44 PM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de> wrote:
>>>
>>> Thanks for the fast feedback, I am going to commit this to the Inbox!
>>>
>>>
>>> > Making it an error might also make smalltalkCI (through SUnit) catch it to continue. Failing the tests rather than halting the run.
>>>
>>> Exactly, that was also my original motivation to ask this question :-)
>>>
>>> Best,
>>> Christoph
>>> ________________________________
>>> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Eliot Miranda <eliot.miranda at gmail.com>
>>> Gesendet: Donnerstag, 9. April 2020 17:03:59
>>> An: The general-purpose Squeak developers list
>>> Betreff: Re: [squeak-dev] Why is ModificationForbidden not an Error?
>>>
>>> Hi Christoph,
>>>
>>>
>>> On Apr 9, 2020, at 7:01 AM, Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de> wrote:
>>>
>>> 
>>>
>>> Hi all,
>>>
>>>
>>> please take a short look at this behavior:
>>>
>>>
>>> TestRunner openForSuite: MorphicUIManagerTest suite. "Run Selected"
>>>
>>>
>>> At the moment (#19541), a bug in FileList2 class >> #endingSpecs (which I'm going to fix ASAP) breaks the test #testShowAllBinParts. But because ModificationForbidden is not an Error, it is not caught by the TestRunner so the whole suite run blows up, too.
>>> I could make another example by triggering a ModificationForbidden in a #drawOn: method to destroy the whole image, too.
>>> (Morph newSubclass compile: 'drawOn: x #(1) at: 1 put: 2'; new) openInHand)
>>>
>>> So my question is: Why doesn't ModificationForbidden derive from Error? Isn't it actually an error? Similar illegal operations such as "#() at: 0" or "{} at: 1 put: #foo" raise some kind of Error, too. Everything I learned so far about Squeak's exception framework tells me that you should have a very good reason if you design an exception neither to derive from Error, nor from Notification. At the moment, we only do have 9 exceptions (bad pun ...) to this rule (and I'm not even sure whether MCNoChangesException couldn't be a notification, and Abort does not even have senders in the Trunk).
>>> I'd be happy if you could give me some pointers on why ModificationForbidden does not follow this rule. How can we deal with this in order to fix the bugs/unexpected behaviors mentioned above?
>>>
>>>
>>> I think this is an oversight in my part.  I agree that ModificationForbidden is an error.  I took the code from Pharo and didn’t notice ModificationForbidden was a Notification.
>>>
>>> Please feel free to make it an Error.
>>>
>>>
>>> Possibly related stuff:
>>>
>>> http://forum.world.st/Squeak-s-AssertionFailure-vs-SUnit-s-TestFailure-td5106818.html
>>> http://forum.world.st/The-Trunk-Kernel-eem-1294-mcz-td5112196.html
>>> http://forum.world.st/The-Trunk-Kernel-eem-1317-mcz-tp5113273p5113433.html
>>>
>>>
>>> Best,
>>> Christoph
>>>
>>>
>>
>