FSX / momoko

Wraps (asynchronous) Psycopg2 for Tornado.
http://momoko.61924.nl/
Other
359 stars 73 forks source link

momoko makes IOError in ioloop #133

Closed selam closed 8 years ago

selam commented 9 years ago

While try to not existed database with not existed user ioloop raise IOError, momoko silently pass the connection error and file descriptor not exists at the time, and we can't catch this exception in our application.

we find a solution for this problem changing lines inside of momoko.connection.py between 706 and 714,

            try:
                if state == POLL_OK:
                    self.ioloop.remove_handler(self.fileno)
                    future.set_result(result)
                elif state == POLL_READ:
                    self.ioloop.update_handler(self.fileno, IOLoop.READ)
                elif state == POLL_WRITE:
                    self.ioloop.update_handler(self.fileno, IOLoop.WRITE)
            except (IOError, OSError):
                future.set_exception(psycopg2.OperationalError("poll() returned %s" % state))

Exception: [E 151114 15:58:16 ioloop:612] Exception in callback (10, <function null_wrapper at 0x7f012456cb90>) Traceback (most recent call last): File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/tornado/ioloop.py", line 866, in start handler_func(fd_obj, events) File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/tornado/stack_context.py", line 275, in null_wrapper return fn(_args, *_kwargs) File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/momoko/connection.py", line 712, in _io_callback self.ioloop.update_handler(self.fileno, IOLoop.WRITE) File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/tornado/ioloop.py", line 708, in update_handler self._impl.modify(fd, events | self.ERROR) IOError: [Errno 2] No such file or directory

haizaar commented 9 years ago

Thanks for the report. I hopefully fixed this several weeks ago. Can you please try the latest master?

Another question - why do you suggest catching OSError as well?

Finally, I'd like to cover it with unittest. Do you have a suggestion on how to write it? On Nov 14, 2015 16:16, "Timu Eren" notifications@github.com wrote:

While try to not existed database with not existed user ioloop raise IOError, momoko silently pass the connection error and file descriptor not exists at the time, and we can't catch this exception in our application.

we find a solution for this problem changing lines inside of momoko.connection.py between 706 and 714,

        try:
            if state == POLL_OK:
                self.ioloop.remove_handler(self.fileno)
                future.set_result(result)
            elif state == POLL_READ:
                self.ioloop.update_handler(self.fileno, IOLoop.READ)
            elif state == POLL_WRITE:
                self.ioloop.update_handler(self.fileno, IOLoop.WRITE)
        except (IOError, OSError):
            future.set_exception(psycopg2.OperationalError("poll() returned %s" % state))

Exception: [E 151114 15:58:16 ioloop:612] Exception in callback (10, ) Traceback (most recent call last): File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/tornado/ioloop.py", line 866, in start handler_func(fd_obj, events) File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/tornado/stack_context.py", line 275, in null_wrapper return fn(_args, *_kwargs) File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/momoko/connection.py", line 712, in _io_callback self.ioloop.update_handler(self.fileno, IOLoop.WRITE) File "/home/timu/workspace/video-ad-server/frontend/.env/local/lib/python2.7/site-packages/tornado/ioloop.py", line 708, in update_handler self._impl.modify(fd, events | self.ERROR) IOError: [Errno 2] No such file or directory

— Reply to this email directly or view it on GitHub https://github.com/FSX/momoko/issues/133.

selam commented 9 years ago

I try with master and i corrected this issue fixed,

Actually i dont have good reason to catch OSError, i copy/paste this catch block from somewhere else.

i dont have to any suggestion for unit testing, but may be this is help:

I use pool object but probably is the same with single connection object

def testIOError(self):
    self.ioloop.add_future(pool_instance_with_bad_dsn.connect(), self.is_executed)

def is_executed(self, future):
      self.assert(True)
haizaar commented 9 years ago

Great. I'll try to release the new version based on the master during the upcoming week.

On Sat, Nov 14, 2015 at 11:12 PM, Timu Eren notifications@github.com wrote:

I try with master and i corrected this issue fixed,

Actually i dont have good reason to catch OSError, i copy/paste this catch block from somewhere else.

i dont have to any suggestion for unit testing, but may be this is help:

I use pool object but probably is the same with single connection object

def testIOError(self): self.ioloop.add_future(pool_instance_with_bad_dsn.connect(), self.is_executed) def is_executed(self, future): self.assert(True)

— Reply to this email directly or view it on GitHub https://github.com/FSX/momoko/issues/133#issuecomment-156746233.

Zaar