Calysto / metakernel

Jupyter/IPython Kernel Tools
BSD 3-Clause "New" or "Revised" License
347 stars 85 forks source link

Doesn't report error status in `execute_reply` msg #175

Open jayendra13 opened 6 years ago

jayendra13 commented 6 years ago

Version info

matlab        : 9.5.0.944444 (R2018b)
matlab_kernel : 0.15.1

I am running a matlab code with matlab_kernel having a syntax error, and tracking the messages from the iopub channel. This is the execute_reply msg from the iopub channel.

{'buffers': [],
 'content': {u'execution_count': 1,
             u'payload': [],
             u'status': u'ok',
             u'user_expressions': {}},
 'header': {u'date': datetime.datetime(2018, 10, 4, 6, 11, 22, 642374, tzinfo=tzutc()),
            u'msg_id': u'1f698f2d-0d99bf43400c21a4a318708d',
            u'msg_type': u'execute_reply',
            u'session': u'94bae730-e12ee227be016e5e947bf704',
            u'username': u'jayendra',
            u'version': u'5.3'},
 'metadata': {u'started': u'2018-10-04T06:11:22.406073Z'},
 'msg_id': u'1f698f2d-0d99bf43400c21a4a318708d',
 'msg_type': u'execute_reply',
 'parent_header': {u'date': datetime.datetime(2018, 10, 4, 6, 11, 16, 937573, tzinfo=tzutc()),
                   u'msg_id': u'e5c2386c-c924e5654ae5e26a59336730',
                   u'msg_type': u'execute_request',
                   u'session': u'3851d47e-5fda30d441f1814fdb335ba5',
                   u'username': u'jayendra',
                   u'version': u'5.3'}}

When matlab encounter the error while executing the code, the status (under the content) should be error instead of ok.

ref: https://jupyter-client.readthedocs.io/en/stable/messaging.html#request-reply

dsblank commented 6 years ago

Thanks for the report! I guess we'll have to inspect the output, looking for text that indicates that the command has failed. I don't have a Matlab license, so we'll either need some examples of what failing code looks like, or someone to submit a PR.

jayendra13 commented 6 years ago

This stream msg would help to identify the error, note the stderr inside the content.

{'buffers': [],
 'content': {u'name': u'stderr',
             u'text': u'\x1b[0;31mError: This statement is incomplete.\n\n\x1b[0m'},
 'header': {u'date': datetime.datetime(2018, 10, 4, 11, 7, 11, 925105, tzinfo=tzutc()),
            u'msg_id': u'ae0f8e8d-fb5257f1bab440ad8bc66045',
            u'msg_type': u'stream',
            u'session': u'4265b9d2-421472de0dd6c27018d449f1',
            u'username': u'jayendra',
            u'version': u'5.3'},
 'metadata': {},
 'msg_id': u'ae0f8e8d-fb5257f1bab440ad8bc66045',
 'msg_type': u'stream',
 'parent_header': {u'date': datetime.datetime(2018, 10, 4, 11, 7, 6, 341136, tzinfo=tzutc()),
                   u'msg_id': u'f23dd5f3-500af3c70424254c974c5be6',
                   u'msg_type': u'execute_request',
                   u'session': u'8931edbb-bebf82a3b871155c66129acd',
                   u'username': u'jayendra',
                   u'version': u'5.3'}}

I can share the whole output if you need it.

dsblank commented 6 years ago

We could just declare that any output on stderr indicates that there was an error (especially if there were also no stdout output). That may not be 100% correct as one could (maybe?) output to stderr without it being an actual error?

dsblank commented 6 years ago

Looks like that is what check_exitcode is for: https://github.com/Calysto/metakernel/blob/0d5dafeec49e3cf68d5d9ab63bff6cf98a7d3664/metakernel/process_metakernel.py#L116-L120

jayendra13 commented 6 years ago

yes output from stderr doesn't always indicate a error, for example fprintf(2,'Hello') would write to stderr, and it is not erroneous code. We can't use output from stderr as flag for error!!

I don't know how check_exitcode would be called.

dsblank commented 6 years ago

check_exitcode is already called, but in matlab_kernel we can override it to look for whatever is the clue that an error just occured. I just don't know what that clue would be.

jayendra13 commented 6 years ago

Can you help me on debugging that ? Currently I am trying this, but I don't think this is the way to run the kernel.

In [1]: from matlab_kernel.kernel import MatlabKernel

In [2]: kernel = MatlabKernel()

In [3]: kernel.do_execute_direct('magic(4)')
---------------------------------------------------------------------------
IOErrorTraceback (most recent call last)
/usr/lib/python2.7/threading.pyc in __bootstrap(self)
    772         # if a non-daemonic encounters this, something else is wrong.
    773         try:
--> 774             self.__bootstrap_inner()
    775         except:
    776             if self.__daemonic and _sys is None:

/usr/lib/python2.7/threading.pyc in __bootstrap_inner(self)
    812                 if _sys and _sys.stderr is not None:
    813                     print>>_sys.stderr, ("Exception in thread %s:\n%s" %
--> 814                                          (self.name, _format_exc()))
    815                 elif self.__stderr is not None:
    816                     # Do the best job possible w/o a huge amt. of code to

IOError: [Errno 9] Bad file descriptor
dsblank commented 6 years ago

I think you can run the kernel that way (at least I can run other metakernels that way). But perhaps you should use python (rather than ipython) and maybe python3 instead of python2.

jayendra13 commented 6 years ago

same things is happening with python2 and python3

Python 3.6.6 (default, Sep 12 2018, 18:26:19) 
[GCC 8.0.1 20180414 (experimental) [trunk revision 259383]] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from matlab_kernel.kernel import MatlabKernel
>>> kernel = MatlabKernel()
>>> kernel.do_execute_direct('magic(4)')
Exception in thread Thread-2:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/home/jayendra/.local/lib/python3.6/site-packages/matlab_kernel/wurlitzer.py", line 148, in forwarder
    handler(data)
  File "/home/jayendra/.local/lib/python3.6/site-packages/matlab_kernel/wurlitzer.py", line 96, in _handle_stdout
    self._stdout.write(self._decode(data))
  File "/home/jayendra/.local/lib/python3.6/site-packages/metakernel/_metakernel.py", line 631, in Print
    self.send_response(self.iopub_socket, 'stream', stream_content)
  File "/home/jayendra/.local/lib/python3.6/site-packages/ipykernel/kernelbase.py", line 463, in send_response
    return self.session.send(stream, msg_or_type, content, self._parent_header,
AttributeError: 'NoneType' object has no attribute 'send'
Traceback (most recent call last):
Python 2.7.15rc1 (default, Apr 15 2018, 21:51:34) 
[GCC 7.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from matlab_kernel.kernel import MatlabKernel
>>> kernel = MatlabKernel()
>>> kernel.do_execute_direct('magic(4)')
    self.__bootstrap_inner()
  File "/usr/lib/python2.7/threading.py", line 814, in __bootstrap_inner
    (self.name, _format_exc()))
IOError: [Errno 9] Bad file descriptor
jayendra13 commented 6 years ago

I have checked the https://github.com/Calysto/metakernel/issues/149 which is similar to this scenario, except the octave vs matlab distinction. With matlab.engine I think this should be easier to do for matalab case then the octave, because it raises the MatlabExecutionError whenever there is an actual error.

I am trying to understand the flow of execution, and I think it is like _metakernel.MetaKernel.do_execute --> kernel.MatlabKernel.do_execute_direct. The do_execute method here https://github.com/Calysto/metakernel/blob/0d5dafeec49e3cf68d5d9ab63bff6cf98a7d3664/metakernel/_metakernel.py#L381 is expecting the retval from the do_execute_direct here https://github.com/Calysto/matlab_kernel/blob/master/matlab_kernel/kernel.py#L84 , which should be object of ExceptionWrapper https://github.com/Calysto/metakernel/blob/0d5dafeec49e3cf68d5d9ab63bff6cf98a7d3664/metakernel/_metakernel.py#L51.

I think this is the issue which needs to be fixed, correct me if I am wrong.

dsblank commented 6 years ago

That sounds right. Thanks for looking into this!