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

Throw JS / Python exceptions according to execution context #11

Closed
GoogleCodeExporter opened this issue Aug 28, 2015 · 20 comments

Comments

@GoogleCodeExporter
Copy link

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Upfront, I know this sounds quite correct, especialy since an uncaught excepton 
will kill an app in python by design, but in JS/Browser/CEF it's a bit 
different, so bare with me...

Here is a trivial example:

We bind this Python function to the browser...
---cut---

def PyFunc(a, b, c):
    return [a, b, c]


--uncut--


And use it like so:

---cut---

var lst = PyFunc(1, 2)


--uncut--

This topples the while app first raising a python exception (logical) and then 
a memory error (possibly related to Issue #2).

Here is the obvious Python error:
'''
TypeError: PyFunc() takes exactly 3 arguments (2 given)
'''

Expected behavior:
- raise a JS exception.

Original issue reported on code.google.com by alex.na...@gmail.com on 28 Aug 2012 at 12:17

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

What about when you call a Python function and pass a js callback to it, and 
that callback fails, should we also raise a JS exception here?

There is more than 1 case, we need to think all options thoroughly.

I am thinking about making this an application option, so that default behavior 
is as it is (most strict), but you can change it through app option so that JS 
exception is raised instead.

When we raise that JS exception, do you have a plan of how you will be notified 
about it? You can miss errors if you don't check DEVeloper Tools. There is an 
example on how to catch all js errors, see file: ceferror.js.

Yes, the memory error is probably because of Issue 2 as we exit app and CEF 
destructor is called that causes memory error, to fix it overwrite 
sys.excepthook with this code:

http://code.google.com/p/cefpython/source/browse/cefpython.pyx#69

But add os.kill(os.getpid(), 9) instead of os._exit(1).

Original comment by czarek.t...@gmail.com on 28 Aug 2012 at 12:41

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Sounds nice-ish, but...

My motive was to try and support each programming model and thus the *expected* 
behavior on it's respective side of the interface as close as possible. and 
here, the two sides are quite different: the Python side is strict and very 
explicit while JS has a more relaxed approach to things, regardless of my 
opinion on either, that is what a programmer is going to expect, and what I 
expected to get :)

So I'd go for the "action origin responsibility" approach -- the action 
initiator gets what is expected in his context from his actions.


So we can split the CEFPython context into two distinct cases:

1) Simple binding call in any direction
The caller will get what is expected on his side. i.e. if I call a JS function 
from python the JS error gets wrapped into a python exception (preferably 
appropriate type of exception), if the caller is V8, then raise and log the 
python exception on the python's side and throw a JS error.

2) "Foreign code" (JS callback, event handler, etc)
IMO this just as well fits the proposed model of responsibility, the caller 
gets the error in his customary way, so if the callback that's called by the 
python side breaks, wrap things into python exception and let the caller deal 
with it. and on the JS side that passed the callback, or bound the handler in 
the first place, will get what it always gets in the JS context -- partial 
execution side-effects (unless you have clean sandboxing with STM, which we do 
not have in general) + total silence for the user and console.error(...) for 
the developer, it he/she uses dev-tools, as you said.

Yes, this is what makes debuging JS hard, but we are not fixing the language 
here, rather the goal is to comply with POLS (Principle of Least Surprise) on 
both sides of the fence of the lib ;)

Original comment by alex.na...@gmail.com on 28 Aug 2012 at 2:01

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

> if the caller is V8, then raise and log the python exception on the python's 
side and throw a JS error.

If I throw a python exception (which will kill application) then what is the 
purpose of  additionaly throwing a JS error? I'm not sure whether I understand 
you.

> the action initiator gets what is expected in his context from his actions.

I am afraid that it is not possible to know who was the action initiator, there 
are more complicated scenarios, the callbacks might be going both directions 
for more than few times and we can't know who started the initial chain of 
actions.

I am having some difficulties understanding you, this is a little complicated. 
I hear about POLS and other concepts for the first time.

Original comment by czarek.t...@gmail.com on 28 Aug 2012 at 2:35

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

I meant that in this case the python exception would not be fatal, it's the 
same situation as raising a python exception in a different thread, it'll kill 
the thread, but not the app.

Let me put it this way:
On one side I'm a JS programmer, and I call a "window.saveState(...)", for 
example, that is implemented in python, if there is a non-critical error, 
something like when an older version of the file already exists I'd expect the 
code to fail in a JavaScripty way, e.g. throw an error or return null or -1, 
not kill the app.

On the python side of this scenario, the traditional way to deal with 
non-critical (as well as most critical) errors is raising exceptions, so 
something like "raise IOError('file already exists')" would be the traditional 
Python-way, but in the current implementation it will kill the while app and 
lose all the user state, unless...

If you implement something specifically for CEFPython, doing things differently 
is bearable, but if you want to directly use a library (like json or pickle or 
some other already running python code, in my case), this will require writing 
wrappers for anything that has the slightest chance of raising something, and 
in Python that's EVERYTHING ;)
...doing so will make the code unnecessarily large, more complex, inflexible, 
error prone and hence harder to maintain. 


> I am afraid that it is not possible to know who was the action initiator...

Yes and no, you are talking about the original entity that started the chain, 
finding it is hard indeed, but I was referring to the immediate initiator, and 
that is trivial...

It's the same model used in python for exception handling, the one calling the 
function will get the error if it fails, and no one cares where he got the 
function from.

Here is an example:
---cut---

def runner(func):
    try:
        return func()
    except Exception, e:
        print e


--uncut--

In this case the same "error passing up the stack strategy" also works -- if 
you do not know how to handle things, then your caller get's the error and so 
on until we reach the top of the stack, and if there's no handler there too in 
python then we kill the app.

In terms of a single callback failing, when there is no call stack, it is 
simpler, there is only one caller regardless of how much the callback got 
passed around, and that is the one making the actual call, and as I said 
before, in terms of a callback call (foreign code, that we might not know the 
origin of) we do not kill anything, just report it and go on living...


--

POLS - Principle of Least Surprise 
(http://en.wikipedia.org/wiki/Principle_of_least_astonishment), the I idea is 
that the behavior that least surprises the user is the best, so any exceptions 
to rules within a context should be avoided at any cost. this is especially 
important in cases like CEFPython when you have two distinct contexts with very 
different cultures coming in contact with each other :)

Original comment by alex.na...@gmail.com on 28 Aug 2012 at 7:54

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Just a single note about python exception killing application: it is happening 
because you have this line in your app:

    sys.excepthook = cefpython.ExceptHook;

You can remove it - and you will get the default behavior of exceptions. 
Application is killed because it calls CefQuitMessageLoop(), see source of 
cefpython.ExceptHook:

def ExceptHook(type, value, traceobject):

    error = "\n".join(traceback.format_exception(type, value, traceobject))
    with open(GetRealPath("error.log"), "a") as file:
        file.write("\n[%s] %s\n" % (time.strftime("%Y-%m-%d %H:%M:%S"), error))
    print("\n"+error+"\n")
    CefQuitMessageLoop()
    CefShutdown()
    os._exit(1) # so that "finally" does not execute

Original comment by czarek.t...@gmail.com on 28 Aug 2012 at 8:54

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

[deleted comment]
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Another note. Python is running mostly on UI thread, the main thread (90% of 
the time),  if there is an exception and this thread is killed then there is no 
point in running the application, because CEF will be dead if UI thread is 
dead, that's why we're killing whole app, otherwise you would just see dead 
windows. CEF will be broken if any one of the threads (UI, IO, FILE) is 
terminated.

If your application creates its own threads and you don't want to kill app when 
one if this threads throws a python exception, then overwrite the except hook 
provided by cefpython, but add code that checks whether exception was thrown on 
one of CEF threads, then you should kill the app:

if cefpython.CurrentlyOn(cefpython.TID_UI) or ...: # or TID_IO or TID_FILE
    KillApp() 

Original comment by czarek.t...@gmail.com on 31 Aug 2012 at 7:09

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Here are some scenarios:

1. Python is calling Javascript - throw Python Exception.

2. Javascript is calling Python (through binding) - throw Javascript Exception.

3. Javascript calls Python Callback:
a) If that is happening during Python > Javascript call then - throw Python 
Exception.
b) If the python callback is saved somewhere and called later from javascript 
context - throw Javascript Exception .

But how do we know whether python callback is called from within Python or 
Javascript context?

4. Python calls Javascript callback:
a) If that is happening during Javascript > Python call then - throw Javascript 
Exception.
b) If that javascript callback was saved somewhere and called later from python 
context - throw Python Exception.

But how do we know whether javascript callback is called from within Javascript 
or Python context?

Original comment by czarek.t...@gmail.com on 31 Aug 2012 at 12:21

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

[deleted comment]
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

We should introduce ExecutionContext object that has 2 methods: 

lockID = AcquireLock(PYTHON_CONTEXT | JAVASCRIPT_CONTEXT)
ReleaseLock(lockID)

AcquireLock() returns an unique identifier of the lock that needs to be later 
passed to ReleaseLock(). The lock will be acquired only if there is no lock 
currently. The lock will be released only if lockID passed is the same as 
global lock.

Example 1: calling Browser.GetMainFrame().ExecuteJavascript(), PYTHON_CONTEXT 
lock is acquired, that is not released until javascript is executed, if during 
that execution some python callback is called from javascript, then before 
executing Python callback a JAVASCRIPT_CONTEXT lock is tried to be acquired, 
but it fails, as there is already a global PYTHON_CONTEXT lock - if someting 
goes wrong during execution of python callback, a Python Exception is thrown.

Example 2: there is currently no lock, GUI loop is running, user does some 
action and a Python callback is executed that was earlier saved in some 
variable. Before executing python callback a lock is acquired with 
JAVASCRIPT_CONTEXT, the lock is acquired successfully as there is no global 
lock currently, if something goes wrong a Javascript Exception is thrown.

Let me know what you think Alex.

Original comment by czarek.t...@gmail.com on 31 Aug 2012 at 12:40

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

There might be some troubles with threading, as Python might get executed on 
different threads. If a lock is acquired in Thread 1 and is not yet released, 
application switches to Thread 2, then ExecutionContext lock that was acquired 
in Thread 1 should have no effect on ExecutionContext of Thread 2.

We should introduce an another parameter to AcquireLock called threadID, so 
that we call it this way:

AcquireLock(PYTHON_CONTEXT, GetCurrentThreadID())

So that lock is linked only to given thread.

Original comment by czarek.t...@gmail.com on 31 Aug 2012 at 12:47

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

I still would like to make it the default of how it works currently, so it's 
easier to debug for someone who is new to CEF. Enabling the ExecutionContext 
feature would be through an option in ApplicationSettings. How would we name 
that option? "enable_execution_context_exceptions"?

Original comment by czarek.t...@gmail.com on 31 Aug 2012 at 12:57

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

[deleted comment]
1 similar comment
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

[deleted comment]
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Original comment by czarek.t...@gmail.com on 1 Sep 2012 at 7:47

  • Changed title: Throw JS / Python exceptions according to execution context
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

I think the way it works now is fine.  If I have an error, I want to know which 
line so I can fix it.  If it's a JS error, my line should be in an HTML or JS 
file.  If it's Python, it should be a PY file.  A Javascript error thrown by a 
PY file error, sounds confusing to me.

Original comment by rich...@gmail.com on 9 Sep 2012 at 7:47

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

The solution proposed in comment #10 will not work in CEF 3, as it uses a
multi-process architecture. CEF 3 is the future, CEF 1 is deprecated now
and receives only maintenance fixes. I don't think I will be developing any 
features that are not compatible with the CEF 3 asynchonous way of working, 
as this will cause troubles when porting CEF 1 apps to CEF 3.

So at the moment I don't have any idea of how this problem could be solved,
knowing nested execution context is not possible in multi-process architecture.

Though I see a solution to the original problem in the first post:

1. Any python function that is binded to javascript - its parameters should be
declared using *args or **kwargs, this way there won't be any error when there
is a wrong number of parameters passed.

2. Embrace python binded function code with try..catch, so that it doesn't throw
a python exception, in the except clause throw a javascript exception.

But how do we throw a javascript exception? I think that we can just use the
Frame.ExecuteJavascript("throw 'some error'") for that. The stack trace of the
error won't be available, but I don't think we can do anything about that, it's
an asynchronous call from a javascript process to a python process.

Although it is not possible to call python synchronously, it is possible to do
it synchronously the other way, javascript can be executed synchronously from
within javascript, see the CefV8Context::Eval() method:

https://code.google.com/p/cefpython/source/browse/cefpython/cef3/include/cef_v8.
h?r=0250b65e046a#208

It is possible to save the javascript stack trace when the python function is
executed, but doing it for all the calls will bring a performance penalty, 
maybe 
it should be configurable for which calls the stack trace should be saved
or to be able to enable it globally for all calls. We would need to change the
way python functions are called, in Issue 33 there is already described on way
of doing this by passing an additional 'context` parameter to each python 
function:

https://code.google.com/p/cefpython/issues/detail?id=33

Or we might just introduce a global variable keeping the stack trace of the last
call from javascript, this way there would be no need to break any existing 
code.

Original comment by czarek.t...@gmail.com on 17 Jul 2013 at 6:55

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

This is not critical, for me at least, I have a workaround working at the 
moment...

Both of your solutions are more of a manual workaround, but considering the 
status of CEF 1 / CEF 3 development, at this point I have no problems with this.

My code is still on CEF1 + I have a parallel/alternative implementation in 
node-webkit which BTW has a similar issue, but with a different, more evil 
twist but that's an OT :)

This is a preliminary opinion as I have not spent too much time thinking about 
it, but a global for context storage seems to be a bad idea epecially in 
parallel code, unless it's not really a global ;) 
...I'll update if my view on this changes.

Original comment by alex.na...@gmail.com on 17 Jul 2013 at 9:13

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

Original comment by czarek.t...@gmail.com on 10 Jan 2014 at 6:24

  • Added labels: Priority-Low
  • Removed labels: Priority-Medium
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Aug 28, 2015

The comment about possibiliy of calling python<>javascript synchronously using 
CefV8Context::Eval() is not valid, as this method can be called only from the 
Renderer process.

Closing issue, as I don't see a way to implement this in CEF 3. It might be 
reopened in future if someone provides a solution.

Original comment by czarek.t...@gmail.com on 10 Aug 2014 at 5:31

  • Changed state: WontFix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.