I think this problem could lead to debugger issues with stepOver and stepThrough. For example: Context>>#stepToHome: ... context := aContext insertSender: (Context contextOn: UnhandledError do: [:ex | ... ]) It is a method which is used for stepThrough. If current method has outer handler for Error then the UnhandledError logic will be ignored. Step over is based on following method: Context>>runUntilErrorOrReturnFrom: aSender ... context := aSender insertSender: (Context contextOn: Error do: [:ex | error ifNil: [ "this is ugly but it fixes the side-effects of not sending an Unhandled error on Halt" error := (ex isKindOf: UnhandledError) ifTrue: [ ex exception ] ifFalse: [ ex ]. ... I think UnhandledError branch will never be true: [ 1/0 ] on: Error do: [:e | e logCr. e3 pass ] Handler here is executed only once with ZeroDivide error. Previous version was based on UnhandledError handler: context := aSender insertSender: (Context contextOn: UnhandledError, Halt do: [:ex | ...] which had the same issue when there is an outer error handler. And we have such general error handler (probably introduced in Pharo 6): WorldMorph>>becomeActiveDuring: aBlock ... aBlock on: Error do: [ :ex | ... ex pass ] Any UI event is processed under this method. ср, 18 дек. 2019 г. в 22:31, Denis Kudriashov <dionisiydk@gmail.com>:

Users of UnhandledError definitely shows that it is a critical bug.

For example we rely on UnhandledError in Announcer to ensure that all subscriptions will be processed independently on errors signalled by any of them:

ann := Announcer new. ann when: ValueChanged do: [:ann | 1 logCr. 1/0 ]. ann when: ValueChanged do: [:ann | 2 logCr. 2/0 ]. ann when: ValueChanged do: [:ann | 3 logCr. 3/0 ].

ann announce: ValueChanged new

It will show 1, 2, 3 in transcript and open 3 debuggers. Each error is deferred to the background process allowing the delivery to continue:

AnnouncementSubscription>>deliver: anAnnouncement

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

^ (self handlesAnnouncement: anAnnouncement ) ifTrue: [

[action cull: anAnnouncement cull: announcer]

on: UnhandledError fork: [:ex | ex pass ]]

Now if you will try to wrap #announce: into handler block the deliver will be interrupted by first error:

[ann announce: ValueChanged new] on: ZeroDivide do: [ :err | err logCr. err pass ].

It will open single debugger at first handler error.

ср, 18 дек. 2019 г. в 20:44, Denis Kudriashov <dionisiydk@gmail.com>:

...

Hi.

I played a bit with exceptions trying to detect that given block will open the debugger. My idea was to simply catch UnhandledError which normally means that no outer code handles given error and therefore debugger is appeared. For my surprise the following example works from playground but not from the browser editor:

[MyTestError signal ] on: UnhandledError do: [ :e | self halt ]

In playground you will see the halt. But from the browser the debugger will show MyTestError.

After breaking my head I found that there is a difference how scripts are executed in those tools. In Playground it is a deferred action. But in the browser it is evaluated as a deep event processing code (keymap processing) and there is an outer error handler (on: Error do: ) which does some work and passes the exception further (err pass). Following script demonstrates the difference in the error processing logic when there is an outer handler:

[

[MyTestError signal ] on: UnhandledError do: [ :e | self halt ]

] on: MyTestError do: [ :z | z pass]

Try it from playground. Second line separately will show the halt while all together it will stop at MyTestError signal.

Debugging shows that when the outer handler is processed it sets the handler context into the exception and it skips the existing handler for UnhandledError.

The question: is it a feature or a bug? Think also how following code should work (unrelated to UnhandledError logic):

[

[ 1/0 ] on: MyTestError do: [ :e | self halt ]

] on: ZeroDivide do: [ :z | MyTestError signal ]

Currently it will show MyTestError signal.

Best regards, Denis

The solution to problem is to resignal UnhandledError instead of simple signalling. Resignal uses original signalContext as a starting point for handlers lookup: UnhandledError>>signalForException: anError ^anError resignalAs: (self new exception: anError; yourself) I played with some of debugger issues and this code really fixes them. For example try debug following script in playground: [ self methodWithError ] on: Error do: [ :e | e logCr. e pass ] With error method: Object>>methodWithError 1/0 Step into #methodWithError and then stepThrough "1/0". In current image it will hands for a while and then it will open another debugger with single error item. The current debugger process will be broken. Now try to apply proposed change for UnhandledError>>signalForException: and repeat experiment. StepThrough "1/0" will move debugger to ZeroDivide signal showing correct stack. Notice that debugging separate "self methodWithError" without outer handler works correctly in both cases. But you should be in clean stack without hidden outer handlers. And in Playground or when you use debugIt it is clean. But you can try in "bad" environment. Execute in browser editor "self halt. self methodWithError". And in debugger just Step Through #methodWithError. It will hangs. Proposed change fixes that. I will prepare PR with tests. But it would be nice to find other broken places which will be fixed чт, 19 дек. 2019 г. в 00:42, Denis Kudriashov <dionisiydk@gmail.com>:

I think this problem could lead to debugger issues with stepOver and stepThrough. For example:

Context>>#stepToHome: ... context := aContext insertSender: (Context contextOn: UnhandledError do: [:ex | ... ])

It is a method which is used for stepThrough. If current method has outer handler for Error then the UnhandledError logic will be ignored.

Step over is based on following method:

Context>>runUntilErrorOrReturnFrom: aSender ...

context := aSender insertSender: (Context

contextOn: Error do: [:ex |

error ifNil: [

"this is ugly but it fixes the side-effects of not sending an Unhandled error on Halt"

error := (ex isKindOf: UnhandledError) ifTrue: [ ex exception ] ifFalse: [ ex ]. ...

I think UnhandledError branch will never be true:

[ 1/0 ] on: Error do: [:e | e logCr. e3 pass ]

Handler here is executed only once with ZeroDivide error. Previous version was based on UnhandledError handler:

context := aSender insertSender: (Context contextOn: UnhandledError, Halt do: [:ex | ...]

which had the same issue when there is an outer error handler. And we have such general error handler (probably introduced in Pharo 6):

WorldMorph>>becomeActiveDuring: aBlock

...

aBlock

on: Error

do: [ :ex | ... ex pass ]

Any UI event is processed under this method.

ср, 18 дек. 2019 г. в 22:31, Denis Kudriashov <dionisiydk@gmail.com>:

...

Users of UnhandledError definitely shows that it is a critical bug.

For example we rely on UnhandledError in Announcer to ensure that all subscriptions will be processed independently on errors signalled by any of them:

ann := Announcer new. ann when: ValueChanged do: [:ann | 1 logCr. 1/0 ]. ann when: ValueChanged do: [:ann | 2 logCr. 2/0 ]. ann when: ValueChanged do: [:ann | 3 logCr. 3/0 ].

ann announce: ValueChanged new

It will show 1, 2, 3 in transcript and open 3 debuggers. Each error is deferred to the background process allowing the delivery to continue:

AnnouncementSubscription>>deliver: anAnnouncement

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

^ (self handlesAnnouncement: anAnnouncement ) ifTrue: [

[action cull: anAnnouncement cull: announcer]

on: UnhandledError fork: [:ex | ex pass ]]

Now if you will try to wrap #announce: into handler block the deliver will be interrupted by first error:

[ann announce: ValueChanged new] on: ZeroDivide do: [ :err | err logCr. err pass ].

It will open single debugger at first handler error.

ср, 18 дек. 2019 г. в 20:44, Denis Kudriashov <dionisiydk@gmail.com>:

...

Hi.

I played a bit with exceptions trying to detect that given block will open the debugger. My idea was to simply catch UnhandledError which normally means that no outer code handles given error and therefore debugger is appeared. For my surprise the following example works from playground but not from the browser editor:

[MyTestError signal ] on: UnhandledError do: [ :e | self halt ]

In playground you will see the halt. But from the browser the debugger will show MyTestError.

After breaking my head I found that there is a difference how scripts are executed in those tools. In Playground it is a deferred action. But in the browser it is evaluated as a deep event processing code (keymap processing) and there is an outer error handler (on: Error do: ) which does some work and passes the exception further (err pass). Following script demonstrates the difference in the error processing logic when there is an outer handler:

[

[MyTestError signal ] on: UnhandledError do: [ :e | self halt ]

] on: MyTestError do: [ :z | z pass]

Try it from playground. Second line separately will show the halt while all together it will stop at MyTestError signal.

Debugging shows that when the outer handler is processed it sets the handler context into the exception and it skips the existing handler for UnhandledError.

The question: is it a feature or a bug? Think also how following code should work (unrelated to UnhandledError logic):

[

[ 1/0 ] on: MyTestError do: [ :e | self halt ]

] on: ZeroDivide do: [ :z | MyTestError signal ]

Currently it will show MyTestError signal.

Best regards, Denis

Hi Denis, Thanks very much for the clarification in your previous email (which I haven't quoted here), I did indeed misunderstand the point you were making. On Thu, 19 Dec 2019 at 19:40, Denis Kudriashov <dionisiydk@gmail.com> wrote:

The solution to problem is to resignal UnhandledError instead of simple signalling. Resignal uses original signalContext as a starting point for handlers lookup:

UnhandledError>>signalForException: anError

^anError resignalAs: (self new

exception: anError;

yourself)

I played with some of debugger issues and this code really fixes them. For example try debug following script in playground:

[ self methodWithError ] on: Error do: [ :e | e logCr. e pass ]

With error method:

Object>>methodWithError

1/0

Step into #methodWithError and then stepThrough "1/0". In current image it will hands for a while and then it will open another debugger with single error item. The current debugger process will be broken. Now try to apply proposed change for UnhandledError>>signalForException: and repeat experiment. StepThrough "1/0" will move debugger to ZeroDivide signal showing correct stack.

Confirmed.

Notice that debugging separate "self methodWithError" without outer handler works correctly in both cases. But you should be in clean stack without hidden outer handlers. And in Playground or when you use debugIt it is clean. But you can try in "bad" environment. Execute in browser editor "self halt. self methodWithError". And in debugger just Step Through #methodWithError. It will hangs. Proposed change fixes that.

I will prepare PR with tests. But it would be nice to find other broken places which will be fixed

I thought I'd take a look at VisualWorks (8.3) to see what it does: it has the same behaviour as with your fix above. This opens some other interesting possibilities. Changing some names and reinterpreting your original example: [ result := [ MyResumableError signal ] on: UnhandledError do: [ :e | self doUltimateFallback ] ] on: MyResumableError do: [ :e | e resume: 42 ]. The MyResumableError handler may be way down the call stack. While the code is more than a little contrived, it does demonstrate the possibility of allowing callers to handle exceptions, but then have a fallback if everything else fails. Great detective work! Thanks, Alistair

PR is here https://github.com/pharo-project/pharo/pull/5470 чт, 19 дек. 2019 г. в 21:51, Denis Kudriashov <dionisiydk@gmail.com>:

чт, 19 дек. 2019 г. в 21:43, Denis Kudriashov <dionisiydk@gmail.com>:

...

I checked Squeak. All my examples work consistently there: UnhandledError handler is always triggered. Even more: signalling new error from the handler block passes it from the original signallerContext:

[

...

[ 1/0 ] on: MyTestError do: [ :e | self halt ]

] on: ZeroDivide do: [ :z | MyTestError signal ]

This code halts in Squeak.

Squeak has special "activeHandle" logic in #handleSignal: code:

Context>>handleSignal: exception

"Sent to handler (on:do:) contexts only. If my exception class (first arg) handles exception

and the handler is active then execute my handle block (second arg), otherwise forward

this message to the next handler context. If none left, execute exception's defaultAction

(see nil>>handleSignal:)."

| handlerActive val |

"If the context has been returned from the handlerActive temp var may not be accessible."

handlerActive := stackp >= 3 and: [(self tempAt: 3) == true].

(((self tempAt: 1) handles: exception) and: [handlerActive]) ifFalse:

[^self nextHandlerContext handleSignal: exception].

exception privHandlerContext: self contextTag.

self tempAt: 3 put: false. "disable self while executing handle block"

val := [(self tempAt: 2) cull: exception]

ensure: [self tempAt: 3 put: true].

self return: val "return from self if not otherwise directed in handle block"

This flag is used by VM primitive to lookup next handler. We should just copy this code into Pharo with corresponding tests