Skip to content

Commit

Permalink
Trilogy::ProtocolError should use SQL codes to determine error class
Browse files Browse the repository at this point in the history
Instead of raising all ProtocolErrors as connection errors,
we use the SQL error code to determine the type of error to raise
when handling the Trilogy error in the Ruby bindings.

The mapping of error codes to error classes was taken from the mysql2
implementation: https://github.com/brianmario/mysql2/blob/master/lib/mysql2/error.rb#L13-L27

Note that we don't need to care about the 2xxx errors because those
are raised by the libmysql client, which Trilogy doesn't deal with.

Co-authored-by: Jean Boussier <jean.boussier@shopify.com>
  • Loading branch information
adrianna-chang-shopify and byroot committed Jan 3, 2023
1 parent 6be9b25 commit 16448b1
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 13 deletions.
11 changes: 3 additions & 8 deletions contrib/ruby/ext/trilogy-ruby/cext.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ static ID id_socket, id_host, id_port, id_username, id_password, id_found_rows,
id_write_timeout, id_keepalive_enabled, id_keepalive_idle, id_keepalive_interval, id_keepalive_count,
id_ivar_affected_rows, id_ivar_fields, id_ivar_last_insert_id, id_ivar_rows, id_ivar_query_time, id_password,
id_database, id_ssl_ca, id_ssl_capath, id_ssl_cert, id_ssl_cipher, id_ssl_crl, id_ssl_crlpath, id_ssl_key,
id_ssl_mode, id_tls_ciphersuites, id_tls_min_version, id_tls_max_version, id_multi_statement;
id_ssl_mode, id_tls_ciphersuites, id_tls_min_version, id_tls_max_version, id_multi_statement, id_from_code;

struct trilogy_ctx {
trilogy_conn_t conn;
Expand Down Expand Up @@ -91,12 +91,7 @@ static void handle_trilogy_error(struct trilogy_ctx *ctx, int rc, const char *ms

case TRILOGY_ERR: {
VALUE message = rb_str_new(ctx->conn.error_message, ctx->conn.error_message_len);
VALUE exc = rb_exc_new3(Trilogy_ProtocolError,
rb_sprintf("%" PRIsVALUE ": %d %" PRIsVALUE, rbmsg, ctx->conn.error_code, message));

rb_ivar_set(exc, rb_intern("@error_code"), INT2FIX(ctx->conn.error_code));
rb_ivar_set(exc, rb_intern("@error_message"), message);

VALUE exc = rb_funcall(Trilogy_ProtocolError, id_from_code, 2, message, INT2NUM(ctx->conn.error_code));
rb_exc_raise(exc);
}

Expand Down Expand Up @@ -1038,7 +1033,7 @@ void Init_cext()
id_tls_min_version = rb_intern("tls_min_version");
id_tls_max_version = rb_intern("tls_max_version");
id_multi_statement = rb_intern("multi_statement");

id_from_code = rb_intern("from_code");
id_ivar_affected_rows = rb_intern("@affected_rows");
id_ivar_fields = rb_intern("@fields");
id_ivar_last_insert_id = rb_intern("@last_insert_id");
Expand Down
38 changes: 33 additions & 5 deletions contrib/ruby/lib/trilogy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,42 @@ class QueryError < ClientError
class CastError < ClientError
end

class ProtocolError < BaseError

class TimeoutError < Errno::ETIMEDOUT
include ConnectionError
end

class ProtocolError < BaseError
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
1927 => ConnectionError, # ER_CONNECTION_KILLED
}

attr_reader :error_code, :error_message

class << self
def from_code(message, code)
ERROR_CODES.fetch(code, self).new(message, code)
end
end

def initialize(error_message, error_code)
super("#{error_code}: #{error_message}")
@error_code = error_code
@error_message = error_message
end
end

class SSLError < BaseError
Expand All @@ -42,10 +74,6 @@ class ConnectionClosed < IOError
include ConnectionError
end

class TimeoutError < Errno::ETIMEDOUT
include ConnectionError
end

def in_transaction?
(server_status & SERVER_STATUS_IN_TRANS) != 0
end
Expand Down
17 changes: 17 additions & 0 deletions contrib/ruby/test/client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,23 @@ def test_timeout_deadlines
end
end

def test_timeout_error
client_1 = new_tcp_client
client_2 = new_tcp_client

create_test_table(client_1)
client_2.change_db("test")

client_1.query("SET GLOBAL innodb_lock_wait_timeout = 2;")
client_1.query("INSERT INTO trilogy_test (varchar_test) VALUES ('a')")
client_1.query("BEGIN")
client_1.query("SELECT * FROM trilogy_test FOR UPDATE")

assert_raises Trilogy::TimeoutError do
client_2.query("SELECT * FROM trilogy_test FOR UPDATE")
end
end

def test_database_error
client = new_tcp_client

Expand Down

0 comments on commit 16448b1

Please sign in to comment.