MariaDB throws error string not caught by ThinkingSphinx::SphinxError.new_from_mysql

fabrizioq commented 2 years ago

Some time after upgrading some packages on a linux machine, our rails test suite started to fail due to connection errors to Sphinx.

The sphinx callback triggered after destroying a mode were not finding a running sphinx demon. Indeed I run the suite without starting the sphinx demon. However these fails were new to me. (In fact, I had never thought about the sphinx implications about destroying the model!)

Strangely, the same tests did not fail on a mac machine -- with mysql installed.

After some time I realized that MariaDB had changed its connection error message by removing "MySQL": "Can't connect to server on '%-.64s' (%d)"

https://github.com/mariadb-corporation/mariadb-connector-c/commit/a08fd79afd07a75687f6186caf3d6e3f5724764f#diff-8e895496dc9b58b3a7076d64c920b6e739a16b6be753cd24e31ce149b2feb7f3L88

This message is not caught by the case statement inside ThinkingSphinx::SphinxError.new_from_mysql.

class ThinkingSphinx::SphinxError < StandardError
  def self.new_from_mysql(error)
    case error.message

    when /Can't connect to MySQL server/,
      /Communications link failure/,
      /Lost connection to MySQL server/
      replacement = ThinkingSphinx::ConnectionError.new(
        "Error connecting to Sphinx via the MySQL protocol. #{error.message}"
      )
      end
   end
end

I tried changing the case statement and my tests again started to pass (or to fail silently :).

I hope this helps someone. Thanks Pat for your incredible work.

pat commented 2 years ago

Thanks for the debugging on this issue! I've just pushed a fix - 150f2ed - and if CI's happy with it, I'll look at getting a new gem release out soon :)

pat commented 1 year ago

It took me half a year 🤦🏻‍♂️ but finally published a new release (v5.5.0) with this fix. Sorry for the delay!

fabrizioq commented 1 year ago

Awesome, congratulations Pat, thanks for everything.

Have a great year!

On Thu, Dec 29, 2022 at 11:16 PM Pat Allan @.***> wrote:

It took me half a year 🤦🏻‍♂️ but finally published a new release (v5.5.0) with this fix. Sorry for the delay!

— Reply to this email directly, view it on GitHub https://github.com/pat/thinking-sphinx/issues/1227#issuecomment-1367687181, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACSNXMJ5HX3QUYS4NREUXTWPZAXBANCNFSM52XC2HQA . You are receiving this because you authored the thread.Message ID: @.***>

pat / thinking-sphinx

MariaDB throws error string not caught by ThinkingSphinx::SphinxError.new_from_mysql #1227