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

Don't fail deferrables when callbacks raise exceptions #248

Merged
merged 2 commits into from Mar 16, 2012

Conversation

thoughtless
Copy link
Contributor

With the current implementation, any exception raised by an EM callback will be rescue by Mysql2::EM::Client::Watcher, and the deferrable will be failed. While I'm certainly not the most qualified person to make this judgement, I don't think that is very idiomatic. I do know that this has made my life rather difficult in trying to work with these deferrables.

Here is an example:

EM.run do
  client = Mysql2::EM::Client.new
  defer = client.query "INSERT INTO `messages` (`content`) VALUES ('what up?')"
  defer.callback do |result|
    publish_to_queue
  end
  defer.errback do |err|
    LOGGER.error "Problem inserting into database: #{err.inspect}"
  end
end

But what if publish_to_queue fails because my queue server is down? With the current implementation, I'll also see an error in my log file saying that there was a problem inserting into the database; however, the database INSERT worked just fine.

With this patch, the above will not write a message to the log when publish_to_queue raises an exception.

* Only fail the deferrable if the exception came from the client.
@qqshfox
Copy link

qqshfox commented Mar 15, 2012

igrigorik/em-synchrony#71

I have already fixed this in production environment for over 6 months.

PS

0.3.8 (November 9th, 2011)

remove fiber support from mysql2, the code has moved to the em-synchrony gem.

* If there is a MySQL error when using EM, don't succeed the deferrable.
* Thanks to: qqshfox/em-synchrony@3448059
@thoughtless
Copy link
Contributor Author

@qqshfox Are you saying that this shouldn't be part of the mysql2 gem? I would think that one shouldn't be required to use em-synchrony to have this behavior.

Also, thanks for the link. It exposed a bug in my implementation (now fixed).

@qqshfox
Copy link

qqshfox commented Mar 15, 2012

@thoughtless You are right. This has nothing to do with fiber (em-synchonry).

But I never use it w/o fiber... So I just thouht you were playing it with fiber...

@qqshfox
Copy link

qqshfox commented Mar 15, 2012

@thoughtless I think that I should have sent that patch here.

@brianmario
Copy link
Owner

thanks!

brianmario added a commit that referenced this pull request Mar 16, 2012
Don't fail deferrables when callbacks raise exceptions
@brianmario brianmario merged commit 42f7b34 into brianmario:master Mar 16, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants