Smalltalk › Pharo › Pharo Smalltalk Developers

Interesting bug or feature in exception processing logic

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

15 messages Options

Denis Kudriashov

Interesting bug or feature in exception processing logic

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

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

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 <[hidden email]>:

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

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

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 <[hidden email]>:

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 <[hidden email]>:
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

alistairgrant

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

Hi Denis,

This is a really interesting discussion, and I haven't wrapped my head
around it properly yet, but one initial observation:

On Wed, 18 Dec 2019 at 21:45, Denis Kudriashov <[hidden email]> wrote:

>
> 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).

I think the difference here is that in the playground 'self' is nil,
while in the browser 'self' is the class being inspected.
UndefinedObject>>handleSignal: tells the exception to execute its
#defaultAction, which is to signal the UnhandledError, and thus it
evaluates the block.

One other thing to keep in mind, UnhandledError is a subclass of
Exception. It will only catch signals of itself, unlike Error or
Exception, which will catch all their subclasses. So 'MyTestError
signal' can only be caught if something catches it and then signals
UnhandledError. In this case it is the #defaultAction.

HTH,
Alistair

alistairgrant

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

On Wed, 18 Dec 2019 at 21:45, Denis Kudriashov <[hidden email]> wrote:

>
> 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.

It isn't skipping the UnhandledError handler. MyTestError isn't a
subclass of UnhandledError, it's a subclass of Error, so we expect the
first exception handler that is triggered in the code above to be the
MyTestError handler.

>
> The question: is it a feature or a bug?

Setting aside whether it is a feature or a bug, it is clearly the
defined behaviour.

> 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 ]

In this case the behaviour should be the same for both the browser and
the playground: the 0 divide exception handler catches 1/0 and signals
MyTestError, which doesn't have a handler, and a debugger is opened.

Cheers,
Alistair

alistairgrant

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

On Wed, 18 Dec 2019 at 23:32, Denis Kudriashov <[hidden email]> wrote:

>
> 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.

This looks like a bug in AnnouncementSubscription>>deliver:. The
exception handler should probably be on Exception since the delivery
mechanism just wants to continue and let something else deal with the
exception.

Cheers,
Alistair

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

In reply to this post by alistairgrant

Hi Alistair.

чт, 19 дек. 2019 г. в 07:10, Alistair Grant <[hidden email]>:

I think the difference here is that in the playground 'self' is nil,
while in the browser 'self' is the class being inspected.
UndefinedObject>>handleSignal: tells the exception to execute its
#defaultAction, which is to signal the UnhandledError, and thus it
evaluates the block.

#handleSignal: is sent to contexts chain in the stack during error processing which ends up at top context and its nil sender. It is not related to the receiver of messages on the stack.

One other thing to keep in mind, UnhandledError is a subclass of
Exception.

That's correct. But it does not matter for my findings.

It will only catch signals of itself, unlike Error or
Exception, which will catch all their subclasses. So 'MyTestError
signal' can only be caught if something catches it and then signals
UnhandledError. In this case it is the #defaultAction.

Yes, and here is the trick. When error resignals UnhandledError from the defaultAction it is caught by UnhandledError handler but only if there was no outer handler which already processed original error

HTH,
Alistair

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

чт, 19 дек. 2019 г. в 00:42, Denis Kudriashov <[hidden email]>:

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.

Sorry, the mistake is here. It's better to copy scripts to the mail. It should be with Exception. But the result is same - single message in transcript:

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

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

In reply to this post by alistairgrant

Hi Alistair

чт, 19 дек. 2019 г. в 07:19, Alistair Grant <[hidden email]>:

On Wed, 18 Dec 2019 at 21:45, Denis Kudriashov <[hidden email]> wrote:
>
> 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.

It isn't skipping the UnhandledError handler. MyTestError isn't a
subclass of UnhandledError, it's a subclass of Error, so we expect the
first exception handler that is triggered in the code above to be the
MyTestError handler.

Hm. It forces me to think that I did not explain the problem.

If your sentence is correct then the following code would not halt:

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

Notice that I use fork to avoid any "garbage" of outer handlers from the doIt processing machinery.

With the handler we #pass the caught error (on: MyTestError do: [ :z | z pass]) which should be equivalent to "not caught scenario" (it should be to my view) but it doesn't (there will be MyTestError instead of halt).

>
> The question: is it a feature or a bug?

Setting aside whether it is a feature or a bug, it is clearly the
defined behaviour.

Do you still think it is clear semantics?

> 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 ]

In this case the behaviour should be the same for both the browser and
the playground: the 0 divide exception handler catches 1/0 and signals
MyTestError, which doesn't have a handler, and a debugger is opened.

Cheers,
Alistair

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

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 <[hidden email]>:

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 <[hidden email]>:
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 <[hidden email]>:
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

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

I will collect here other issues if I will find them.

Looking in Context methods which I mentioned here I found that debugger is broken for stepping through warnings. And it means that debugging deprecated methods can hang debugger.

I will create another thread on this issue because it has more questions on current exceptions design.

чт, 19 дек. 2019 г. в 18:39, Denis Kudriashov <[hidden email]>:

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 <[hidden email]>:
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 <[hidden email]>:
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 <[hidden email]>:
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

alistairgrant

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

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 <[hidden email]> 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

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

In reply to this post by Denis Kudriashov

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.

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

чт, 19 дек. 2019 г. в 21:43, Denis Kudriashov <[hidden email]>:

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

Denis Kudriashov

Re: Interesting bug or feature in exception processing logic

PR is here https://github.com/pharo-project/pharo/pull/5470

чт, 19 дек. 2019 г. в 21:51, Denis Kudriashov <[hidden email]>:

чт, 19 дек. 2019 г. в 21:43, Denis Kudriashov <[hidden email]>:
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