Skip to content

More specific exception classes #911

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

Merged
merged 3 commits into from
Nov 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ext/mysql2/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "mysql_enc_name_to_ruby.h"

VALUE cMysql2Client;
extern VALUE mMysql2, cMysql2Error;
extern VALUE mMysql2, cMysql2Error, cMysql2TimeoutError;
static VALUE sym_id, sym_version, sym_header_version, sym_async, sym_symbolize_keys, sym_as, sym_array, sym_stream;
static VALUE sym_no_good_index_used, sym_no_index_used, sym_query_was_slow;
static ID intern_brackets, intern_merge, intern_merge_bang, intern_new_with_args;
Expand Down Expand Up @@ -660,7 +660,7 @@ static VALUE do_query(void *args) {
retval = rb_wait_for_single_fd(async_args->fd, RB_WAITFD_IN, tvp);

if (retval == 0) {
rb_raise(cMysql2Error, "Timeout waiting for a response from the last query. (waited %d seconds)", FIX2INT(read_timeout));
rb_raise(cMysql2TimeoutError, "Timeout waiting for a response from the last query. (waited %d seconds)", FIX2INT(read_timeout));
}

if (retval < 0) {
Expand Down
3 changes: 2 additions & 1 deletion ext/mysql2/mysql2_ext.c
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
#include <mysql2_ext.h>

VALUE mMysql2, cMysql2Error;
VALUE mMysql2, cMysql2Error, cMysql2TimeoutError;

/* Ruby Extension initializer */
void Init_mysql2() {
mMysql2 = rb_define_module("Mysql2");
cMysql2Error = rb_const_get(mMysql2, rb_intern("Error"));
cMysql2TimeoutError = rb_const_get(cMysql2Error, rb_intern("TimeoutError"));

init_mysql2_client();
init_mysql2_result();
Expand Down
2 changes: 1 addition & 1 deletion ext/mysql2/statement.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <mysql2_ext.h>

VALUE cMysql2Statement;
extern VALUE mMysql2, cMysql2Error, cBigDecimal, cDateTime, cDate;
extern VALUE mMysql2, cMysql2Error, cMysql2TimeoutError, cBigDecimal, cDateTime, cDate;
static VALUE sym_stream, intern_new_with_args, intern_each, intern_to_s, intern_merge_bang;
static VALUE intern_sec_fraction, intern_usec, intern_sec, intern_min, intern_hour, intern_day, intern_month, intern_year;

Expand Down
51 changes: 43 additions & 8 deletions lib/mysql2/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,60 @@ class Error < StandardError
replace: '?'.freeze,
}.freeze

ConnectionError = Class.new(Error)
TimeoutError = Class.new(Error)

CODES = {
1205 => TimeoutError, # ER_LOCK_WAIT_TIMEOUT

1044 => ConnectionError, # ER_DBACCESS_DENIED_ERROR
1045 => ConnectionError, # ER_ACCESS_DENIED_ERROR
1152 => ConnectionError, # ER_ABORTING_CONNECTION
1153 => ConnectionError, # ER_NET_PACKET_TOO_LARGE
1154 => ConnectionError, # ER_NET_READ_ERROR_FROM_PIPE
1155 => ConnectionError, # ER_NET_FCNTL_ERROR
1156 => ConnectionError, # ER_NET_PACKETS_OUT_OF_ORDER
1157 => ConnectionError, # ER_NET_UNCOMPRESS_ERROR
1158 => ConnectionError, # ER_NET_READ_ERROR
1159 => ConnectionError, # ER_NET_READ_INTERRUPTED
1160 => ConnectionError, # ER_NET_ERROR_ON_WRITE
1161 => ConnectionError, # ER_NET_WRITE_INTERRUPTED

2001 => ConnectionError, # CR_SOCKET_CREATE_ERROR
2002 => ConnectionError, # CR_CONNECTION_ERROR
2003 => ConnectionError, # CR_CONN_HOST_ERROR
2004 => ConnectionError, # CR_IPSOCK_ERROR
2005 => ConnectionError, # CR_UNKNOWN_HOST
2006 => ConnectionError, # CR_SERVER_GONE_ERROR
2007 => ConnectionError, # CR_VERSION_ERROR
2009 => ConnectionError, # CR_WRONG_HOST_INFO
2012 => ConnectionError, # CR_SERVER_HANDSHAKE_ERR
2013 => ConnectionError, # CR_SERVER_LOST
2020 => ConnectionError, # CR_NET_PACKET_TOO_LARGE
2026 => ConnectionError, # CR_SSL_CONNECTION_ERROR
2027 => ConnectionError, # CR_MALFORMED_PACKET
2047 => ConnectionError, # CR_CONN_UNKNOW_PROTOCOL
2048 => ConnectionError, # CR_INVALID_CONN_HANDLE
2049 => ConnectionError, # CR_UNUSED_1
}.freeze

attr_reader :error_number, :sql_state

# Mysql gem compatibility
alias errno error_number
alias error message

def initialize(msg)
@server_version ||= nil
def initialize(msg, server_version = nil, error_number = nil, sql_state = nil)
@server_version = server_version
@error_number = error_number
@sql_state = sql_state ? sql_state.encode(ENCODE_OPTS) : nil

super(clean_message(msg))
end

def self.new_with_args(msg, server_version, error_number, sql_state)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember there was a reason for all of this, but I don't remember what that reason was 🤕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember git blaming it and figuring out that it wasn't required anymore. Let me blame again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, don't remember why I dimmed it unnecessary #545

I still think it's better this way, but I can remove that change if you prefer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the arguments into the constructor in f9eba44 but moved them out to a separate method in 1d4de89

err = allocate
err.instance_variable_set('@server_version', server_version)
err.instance_variable_set('@error_number', error_number)
err.instance_variable_set('@sql_state', sql_state.encode(ENCODE_OPTS))
err.send(:initialize, msg)
err
error_class = CODES.fetch(error_number, self)
error_class.new(msg, server_version, error_number, sql_state)
end

private
Expand Down
12 changes: 6 additions & 6 deletions spec/mysql2/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
end
end

it "should raise an exception upon connection failure" do
it "should raise a Mysql::Error::ConnectionError upon connection failure" do
expect do
# The odd local host IP address forces the mysql client library to
# use a TCP socket rather than a domain socket.
new_client('host' => '127.0.0.2', 'port' => 999999)
end.to raise_error(Mysql2::Error)
end.to raise_error(Mysql2::Error::ConnectionError)
end

it "should raise an exception on create for invalid encodings" do
Expand Down Expand Up @@ -559,7 +559,7 @@ def run_gc
client = new_client(read_timeout: 0)
expect do
client.query('SELECT SLEEP(0.1)')
end.to raise_error(Mysql2::Error)
end.to raise_error(Mysql2::Error::TimeoutError)
end

# XXX this test is not deterministic (because Unix signal handling is not)
Expand Down Expand Up @@ -918,10 +918,10 @@ def run_gc
end
end

it "should raise a Mysql2::Error exception upon connection failure" do
it "should raise a Mysql2::Error::ConnectionError exception upon connection failure due to invalid credentials" do
expect do
new_client(host: "localhost", username: 'asdfasdf8d2h', password: 'asdfasdfw42')
end.to raise_error(Mysql2::Error)
new_client(host: 'localhost', username: 'asdfasdf8d2h', password: 'asdfasdfw42')
end.to raise_error(Mysql2::Error::ConnectionError)

expect do
new_client(DatabaseCredentials['root'])
Expand Down