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

Exceptions thrown with error.throwWithMessage can not be catched #963

Closed
flbulgarelli opened this issue Sep 1, 2016 · 10 comments
Closed
Assignees
Milestone

Comments

@flbulgarelli
Copy link
Contributor

See the following REPL session and compare how throw and error.throwWithMessage behave differently:

Wollok interactive console (type "quit" to quit):
>>> error
error[]
>>> error.throwWithMessage("foo")
wollok.lang.Exception: foo
    at wollok.lib.error.throwWithMessage(aMessage) [/lib.wlk:255]
    at  [/__synthetic0.wpgm:3]
    at  [/lib.wlk:249]
    at  [/wollokREPL.wlk:1]

>>> try { error.throwWithMessage("foo") } catch e : Exception { } 
wollok.lang.Exception: foo
    at wollok.lib.error.throwWithMessage(aMessage) [/lib.wlk:255]
    at  [/__synthetic0.wpgm:3]
    at  [/__synthetic0.wpgm:3]
    at  [/lib.wlk:249]
    at  [/wollokREPL.wlk:1]

>>> try { throw new Exception("foo") } catch e : Exception { } 
null
>>> try { throw new Exception("foo") } catch e : wollok.lang.Exception { } 
null
>>> try { error.throwWithMessage("foo") } catch e : wollok.lang.Exception { } 
wollok.lang.Exception: foo
    at wollok.lib.error.throwWithMessage(aMessage) [/lib.wlk:255]
    at  [/__synthetic0.wpgm:3]
    at  [/__synthetic0.wpgm:3]
    at  [/__synthetic0.wpgm:3]
    at  [/__synthetic0.wpgm:3]
    at  [/__synthetic0.wpgm:3]
    at  [/lib.wlk:249]
    at  [/wollokREPL.wlk:1]

>>> 

I think that this has sometime worked, but I am not 100% confident about that.

Is this easy to fix? Is there any workaround to this bug? It is somewhat blocking for me since it breaks the mumuki console and prevents to run many exercises.

@javierfernandes
Copy link
Member

There are at least 2 different issues here

  1. Repl shouldn't show output for empty catchs
>>> try { throw new Exception("foo") } catch e : Exception { } 
null

The piece of code doesn't produce any value, so ideally it shouldn't log anything.

But still the execution semantic of the interpreter is right here. You are catching the same exception you are throwing. It must not pop to the repl.

  1. Exception thrown by the "error" object is not being catched !

try { error.throwWithMessage("foo") } catch e : wollok.lang.Exception { }

This should be catched and it shouldn't pop the exception further.
This issues is more important than the first one.
Not really sure why this is happening. Maybe the exceptions is being wrapped.. not sure.

@flbulgarelli
Copy link
Contributor Author

flbulgarelli commented Sep 2, 2016

Yes, the issue I am reporting here is the latter - the first is an issue too, but is is non-blocking, and I think it happens with ifs too.

Just a hint: the second issue doesn't only happen in the REPL, but also though the wollok server, within a program block.

@fdodino
Copy link
Collaborator

fdodino commented Sep 6, 2016

 try { error.throwWithMessage("foo") } catch e : Exception {  } 

While same line in Wollok program works fine, in REPL it throws an exception.
In WollokInterpreter, rootObject are different

rootObject: [org.uqbar.project.wollok.wollokDsl.impl.ImportImpl@4f3faa70 (importedNamespace: wollokREPL.*) 

that's for REPL

`rootObject: [org.uqbar.project.wollok.wollokDsl.impl.WProgramImpl@6a9d5dff (name: abc)] 

that works for a Program.

I'll continue my research.

@fdodino
Copy link
Collaborator

fdodino commented Sep 6, 2016

Ok, problem is that in WollokInterpreterEvaluator

    def dispatch evaluate(WTry t) {
        try
            t.expression.eval
        catch (WollokProgramExceptionWrapper e) {
            val cach = t.catchBlocks.findFirst[ it.matches(e.wollokException)   ]
            if (cach != null) {
                cach.evaluate(e)
            } else
                throw e
        } finally
            t.alwaysExpression?.eval
    }

It calls "matches" extension method:

    def boolean matches(WCatch cach, WollokObject it) {
        cach.exceptionType == null || isKindOf(cach.exceptionType)
    }

and isKindOf is

def isKindOf(WMethodContainer c) { behavior.isKindOf(c) }

But... while in program they are the same class:
behavior: org.uqbar.project.wollok.wollokDsl.impl.WClassImpl@26bab2f1 (name: Exception)
c: org.uqbar.project.wollok.wollokDsl.impl.WClassImpl@6d4c273c (name: Exception)

In REPL they are different classes:
behavior: org.uqbar.project.wollok.wollokDsl.impl.WClassImpl@26bab2f1 (name: Exception)
c: org.uqbar.project.wollok.wollokDsl.impl.WClassImpl@6d4c273c (name: Exception)

So... this method return false, and an exception is thrown:

    def static dispatch isKindOf(WClass c1, WClass c2) {
        WollokModelExtensions.isSuperTypeOf(c2, c1)
    }

WollokModelExtension

    def static boolean isSuperTypeOf(WClass a, WClass b) {
        a == b || (b.parent != null && a.isSuperTypeOf(b.parent))
    }

But I don't get why... I feel like an economic analyst.

@javierfernandes
Copy link
Member

Nice analysis !!

Basically, WClasses equality is currently based on identity. And somehow on the REPL the base classes of wollok (SDK) are being loaded twice (or more times?).
So when it tries to compare two classes they don't match.

I would be good to understand the background fact that is loading SDK many times.
But, another options is to change class equality to something like a value object unrelated to instances.

 def static boolean isSuperTypeOf(WClass a, WClass b) {
        a == b || (b.parent != null && a.isSuperTypeOf(b.parent))
    }

Could be something like

 def static boolean isSuperTypeOf(WClass a, WClass b) {
        a.fqn == b.fqn || (b.parent != null && a.isSuperTypeOf(b.parent))
    }

Notice the "fqn". I'm not sure but I believe that we have one extension method with that name somewhere. It gives you the full element name with package and all.

@fdodino
Copy link
Collaborator

fdodino commented Sep 6, 2016

👍

Yes, I was thinking that as a workaround, but now I'm more confident because you suggested so.
😄

Nevertheless I'll try to see what is going on here a little bit more.

@fdodino
Copy link
Collaborator

fdodino commented Sep 6, 2016

fqn is an valid extension method

@npasserini
Copy link
Member

:)

On Tue, Sep 6, 2016 at 10:18 PM, Fernando Dodino notifications@github.com
wrote:

fqn is an extension method valid


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#963 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEa1OSceV9c6CgG78tJ_-cg-q34sFOo1ks5qncqTgaJpZM4Jy1Tg
.

@fdodino
Copy link
Collaborator

fdodino commented Sep 6, 2016

Well, I fighted and fighted, but I can't figure out where does Wollok create a second instance of Exception class. It happens only with error wko, throwing an error works, so I put a println line in every evaluation of WollokInterpreterEvaluator.

Using fqn comparison it works perfect!

Wollok interactive console (type "quit" to quit):
>>> try { error.throwWithMessage("foo") } catch e : Exception { console.println("jeje") }
jeje
>>> try { error.throwWithMessage("foo") } catch e : Exception { }
null
>>> try { throw new Exception("foo") } catch e : Exception { console.println("jeje") }
jeje

@fdodino
Copy link
Collaborator

fdodino commented Sep 6, 2016

Obviously, second example outputs null, it's not the desired behavior. But we should open a new issue for that.

@fdodino fdodino self-assigned this Sep 6, 2016
@fdodino fdodino added this to the Wollok v1.5.1 milestone Sep 6, 2016
tesonep added a commit that referenced this issue Sep 18, 2016
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

4 participants