Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Library "Catch errors in a script" terminates calling script when used in a reporter. #1941

Open
dlnelson opened this issue Nov 22, 2017 · 8 comments

Comments

@dlnelson
Copy link

When using "safely try . . ." in a reporter, when an error occurs, the error handler runs, but the calling script terminates. I've attached the smallest repro I could find:

ErrorHandlingTest.zip
reporter

Note that running <should report true> at top level displays a report bubble with <true>, but using it in, e.g., an if-statement fails.
caller

Rewriting using a continuation seems to work, but isn't really a solution suitable for beginners:
continuation

@brianharvey
Copy link
Collaborator

brianharvey commented Nov 26, 2017

This works:
errorhandlingtest script pic
The trouble is that the RETURN block in the error handler is returning from the wrong context, I think. Jens is going to tell me that this is what I get for letting 12-year-olds write libraries...

@jguille2
Copy link
Contributor

But the problem here is to mix synchronous and asynchronous actions. I can not build a sync stack with async call inside... I need to call the async action before and send the sync action as a callback.

Let's see:

  • We can define this predicate:
    checkpredicate
  • It seems to be fine
    checkaval
  • But it isn't!
    checknotaval
  • Then, we need to define the checking as an action (not a reporter-predicate)
    checkalternative
  • See that these actions have the same result
    checkactions
  • But be careful! This also has the same result!!
    checkactions2

We can reproduce more problems using async "safely try" into reporters-predicate blocks definition. We must change these cases to actions blocks. Or we must change "safely try" block to a sync operation.

@Naharie
Copy link

Naharie commented Nov 28, 2017

Can anyone tell me what corner cases I am missing? I made a change that makes all the test cases work. Here is the updated project:
ErrorHandlingTest.zip

@jguille2
Copy link
Contributor

Oh @KingDavid12 ,
But you have changed 'safely try' block!!!! 👿

Original block (from distro):
safelydistro
And your modification...
safelymod

Apart from discussing how the blog should be (could be another issue), I think the current behavior and this issue (the project reported) is only a question about sync-async operation.

Joan

@Naharie
Copy link

Naharie commented Nov 28, 2017

@jguille2 Sorry about that, I misunderstood what the focus of the thread was about on my first read through.

@brianharvey
Copy link
Collaborator

I'm confused... @jguille2: If @KingDavid12's modified SAFELY TRY works in all cases, shouldn't we replace the library one with that?

@Naharie
Copy link

Naharie commented Nov 28, 2017

@brianharvey I don't know that it will work in all existing projects, I only tried my modified one on the examples in the thread and a few projects of my own. (Beside the point: Is there an official set of tests for this kind of thing?) So we should probably run a few more test cases before just using it.

@jguille2
Copy link
Contributor

Don't worry @KingDavid12 (I joked after being freaking without knowing why the block suddenly had a different behavior 😄 ... and I like your comments and examples in Snap! )

@brianharvey I have no criteria to comment here about SAFELY TRY modifications. I just commented to help into @dlnelson issue and to show the workaround according to current behavior. Certainly current code is more complex than @KingDavid12 ones, but I don't know exactly the aims of the lib block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants