Skip to content

Commit 7338020

Browse files
committed
properly report stored procedure errors when using info messages
Capturing errors while in a non-blocking state was originally structured to capture a single error. This was intentional in order to avoid capturing more generic info messages that FreeTDS might send before the Global VM Lock was obtained. In most circumstances this is what we want. However, now that we capture info messages it is possible that a info message will be stored and the actual runtime error will be discarded as non-important. The result is that while a runtime error is reported in the database, a TinyTds error is never thrown and only the info message is handled. A subset of this problem is that only one info message can be captured while in non-blocking mode which prevents stored procedures from reporting multiple info messages to TinyTds. To fix this issue, the reported messages are stored within a dynamic array of tinytds_errordata structs, then processed normally once the GVL is obtained. Given the fact that we don't know the number of messages that will be sent, we dynamically manage and re-allocate memory for the nonblocking_errors array as needed. We can't use the ruby C API because it is not safe to call while in a non-blocking state as shown by #133.
1 parent 57b0917 commit 7338020

File tree

5 files changed

+114
-44
lines changed

5 files changed

+114
-44
lines changed

ext/tiny_tds/client.c

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int is_message, int cancel, cons
5757

5858

5959
// Lib Backend (Memory Management & Handlers)
60+
static void push_userdata_error(tinytds_client_userdata *userdata, tinytds_errordata error) {
61+
// reallocate memory for the array as needed
62+
if (userdata->nonblocking_errors_size == userdata->nonblocking_errors_length) {
63+
userdata->nonblocking_errors_size *= 2;
64+
userdata->nonblocking_errors = realloc(userdata->nonblocking_errors, userdata->nonblocking_errors_size * sizeof(tinytds_errordata));
65+
}
66+
67+
userdata->nonblocking_errors[userdata->nonblocking_errors_length] = error;
68+
userdata->nonblocking_errors_length++;
69+
}
6070

6171
int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, char *dberrstr, char *oserrstr) {
6272
static const char *source = "error";
@@ -111,24 +121,16 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c
111121
userdata->dbcancel_sent = 1;
112122
}
113123

114-
/*
115-
If we've already captured an error message, don't overwrite it. This is
116-
here because FreeTDS sends a generic "General SQL Server error" message
117-
that will overwrite the real message. This is not normally a problem
118-
because a ruby exception is normally thrown and we bail before the
119-
generic message can be sent.
120-
*/
121-
if (!userdata->nonblocking_error.is_set) {
122-
userdata->nonblocking_error.is_message = 0;
123-
userdata->nonblocking_error.cancel = cancel;
124-
strncpy(userdata->nonblocking_error.error, dberrstr, ERROR_MSG_SIZE);
125-
strncpy(userdata->nonblocking_error.source, source, ERROR_MSG_SIZE);
126-
userdata->nonblocking_error.severity = severity;
127-
userdata->nonblocking_error.dberr = dberr;
128-
userdata->nonblocking_error.oserr = oserr;
129-
userdata->nonblocking_error.is_set = 1;
130-
}
131-
124+
tinytds_errordata error = {
125+
.is_message = 0,
126+
.cancel = cancel,
127+
.severity = severity,
128+
.dberr = dberr,
129+
.oserr = oserr
130+
};
131+
strncpy(error.error, dberrstr, ERROR_MSG_SIZE);
132+
strncpy(error.source, source, ERROR_MSG_SIZE);
133+
push_userdata_error(userdata, error);
132134
} else {
133135
rb_tinytds_raise_error(dbproc, 0, cancel, dberrstr, source, severity, dberr, oserr);
134136
}
@@ -144,16 +146,21 @@ int tinytds_msg_handler(DBPROCESS *dbproc, DBINT msgno, int msgstate, int severi
144146

145147
// See tinytds_err_handler() for info about why we do this
146148
if (userdata && userdata->nonblocking) {
147-
if (!userdata->nonblocking_error.is_set) {
148-
userdata->nonblocking_error.is_message = !is_message_an_error;
149-
userdata->nonblocking_error.cancel = is_message_an_error;
150-
strncpy(userdata->nonblocking_error.error, msgtext, ERROR_MSG_SIZE);
151-
strncpy(userdata->nonblocking_error.source, source, ERROR_MSG_SIZE);
152-
userdata->nonblocking_error.severity = severity;
153-
userdata->nonblocking_error.dberr = msgno;
154-
userdata->nonblocking_error.oserr = msgstate;
155-
userdata->nonblocking_error.is_set = 1;
156-
}
149+
/*
150+
In the case of non-blocking command batch execution we can receive multiple messages
151+
(including errors). We keep track of those here so they can be processed once the
152+
non-blocking call returns.
153+
*/
154+
tinytds_errordata error = {
155+
.is_message = !is_message_an_error,
156+
.cancel = is_message_an_error,
157+
.severity = severity,
158+
.dberr = msgno,
159+
.oserr = msgstate
160+
};
161+
strncpy(error.error, msgtext, ERROR_MSG_SIZE);
162+
strncpy(error.source, source, ERROR_MSG_SIZE);
163+
push_userdata_error(userdata, error);
157164

158165
if (is_message_an_error && !dbdead(dbproc) && !userdata->closed) {
159166
dbcancel(dbproc);
@@ -171,7 +178,10 @@ static void rb_tinytds_client_reset_userdata(tinytds_client_userdata *userdata)
171178
userdata->dbsqlok_sent = 0;
172179
userdata->dbcancel_sent = 0;
173180
userdata->nonblocking = 0;
174-
userdata->nonblocking_error.is_set = 0;
181+
// the following is mainly done for consistency since the values are reset accordingly in nogvl_setup/cleanup.
182+
// the nonblocking_errors array does not need to be freed here. That is done as part of nogvl_cleanup.
183+
userdata->nonblocking_errors_length = 0;
184+
userdata->nonblocking_errors_size = 0;
175185
}
176186

177187
static void rb_tinytds_client_mark(void *ptr) {

ext/tiny_tds/client.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
void init_tinytds_client();
66

77
#define ERROR_MSG_SIZE 1024
8+
#define ERRORS_STACK_INIT_SIZE 2
89

910
typedef struct {
10-
short int is_set;
1111
int is_message;
1212
int cancel;
1313
char error[ERROR_MSG_SIZE];
@@ -25,7 +25,9 @@ typedef struct {
2525
RETCODE dbsqlok_retcode;
2626
short int dbcancel_sent;
2727
short int nonblocking;
28-
tinytds_errordata nonblocking_error;
28+
short int nonblocking_errors_length;
29+
short int nonblocking_errors_size;
30+
tinytds_errordata *nonblocking_errors;
2931
VALUE message_handler;
3032
} tinytds_client_userdata;
3133

ext/tiny_tds/result.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ static void dbcancel_ubf(DBPROCESS *client) {
8686
static void nogvl_setup(DBPROCESS *client) {
8787
GET_CLIENT_USERDATA(client);
8888
userdata->nonblocking = 1;
89+
userdata->nonblocking_errors_length = 0;
90+
userdata->nonblocking_errors = malloc(ERRORS_STACK_INIT_SIZE * sizeof(tinytds_errordata));
91+
userdata->nonblocking_errors_size = ERRORS_STACK_INIT_SIZE;
8992
}
9093

9194
static void nogvl_cleanup(DBPROCESS *client) {
@@ -95,17 +98,22 @@ static void nogvl_cleanup(DBPROCESS *client) {
9598
Now that the blocking operation is done, we can finally throw any
9699
exceptions based on errors from SQL Server.
97100
*/
98-
if (userdata->nonblocking_error.is_set) {
99-
userdata->nonblocking_error.is_set = 0;
101+
for (short int i = 0; i < userdata->nonblocking_errors_length; i++) {
102+
tinytds_errordata error = userdata->nonblocking_errors[i];
100103
rb_tinytds_raise_error(client,
101-
userdata->nonblocking_error.is_message,
102-
userdata->nonblocking_error.cancel,
103-
userdata->nonblocking_error.error,
104-
userdata->nonblocking_error.source,
105-
userdata->nonblocking_error.severity,
106-
userdata->nonblocking_error.dberr,
107-
userdata->nonblocking_error.oserr);
104+
error.is_message,
105+
error.cancel,
106+
error.error,
107+
error.source,
108+
error.severity,
109+
error.dberr,
110+
error.oserr
111+
);
108112
}
113+
114+
free(userdata->nonblocking_errors);
115+
userdata->nonblocking_errors_length = 0;
116+
userdata->nonblocking_errors_size = 0;
109117
}
110118

111119
static RETCODE nogvl_dbsqlok(DBPROCESS *client) {

test/result_test.rb

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,24 @@ class ResultTest < TinyTds::TestCase
652652
assert_equal 1, messages.length, 'there should be one message after one print statement'
653653
assert_equal msg, m.message, 'message text'
654654
end
655+
656+
it 'must raise an error preceded by a `print` message' do
657+
messages.clear
658+
action = lambda { @client.execute("EXEC tinytds_TestPrintWithError").do }
659+
assert_raise_tinytds_error(action) do |e|
660+
assert_equal 'hello', messages.first.message, 'message text'
661+
662+
assert_equal "Error following print", e.message
663+
assert_equal 16, e.severity
664+
assert_equal 50000, e.db_error_number
665+
end
666+
end
667+
668+
it 'calls the provided message handler for each of a series of `print` messages' do
669+
messages.clear
670+
@client.execute("EXEC tinytds_TestSeveralPrints").do
671+
assert_equal ['hello 1', 'hello 2', 'hello 3'], messages.map { |e| e.message }, 'message list'
672+
end
655673
end
656674

657675
it 'must not raise an error when severity is 10 or less' do
@@ -770,4 +788,3 @@ def insert_and_select_datatype(datatype)
770788
end
771789

772790
end
773-

test/test_helper.rb

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ def load_current_schema
153153
loader.execute(drop_sql).do
154154
loader.execute(schema_sql).do
155155
loader.execute(sp_sql).do
156+
loader.execute(sp_error_sql).do
157+
loader.execute(sp_several_prints_sql).do
156158
loader.close
157159
true
158160
end
@@ -167,7 +169,16 @@ def drop_sql_sybase
167169
) DROP TABLE datatypes
168170
IF EXISTS(
169171
SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestReturnCodes'
170-
) DROP PROCEDURE tinytds_TestReturnCodes|
172+
) DROP PROCEDURE tinytds_TestReturnCodes
173+
IF EXISTS(
174+
SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestPrintWithError'
175+
) DROP PROCEDURE tinytds_TestPrintWithError
176+
IF EXISTS(
177+
SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestPrintWithError'
178+
) DROP PROCEDURE tinytds_TestPrintWithError
179+
IF EXISTS(
180+
SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestSeveralPrints'
181+
) DROP PROCEDURE tinytds_TestSeveralPrints|
171182
end
172183

173184
def drop_sql_microsoft
@@ -181,7 +192,15 @@ def drop_sql_microsoft
181192
IF EXISTS (
182193
SELECT name FROM sysobjects
183194
WHERE name = 'tinytds_TestReturnCodes' AND type = 'P'
184-
) DROP PROCEDURE tinytds_TestReturnCodes|
195+
) DROP PROCEDURE tinytds_TestReturnCodes
196+
IF EXISTS (
197+
SELECT name FROM sysobjects
198+
WHERE name = 'tinytds_TestPrintWithError' AND type = 'P'
199+
) DROP PROCEDURE tinytds_TestPrintWithError
200+
IF EXISTS (
201+
SELECT name FROM sysobjects
202+
WHERE name = 'tinytds_TestSeveralPrints' AND type = 'P'
203+
) DROP PROCEDURE tinytds_TestSeveralPrints|
185204
end
186205

187206
def sp_sql
@@ -191,6 +210,21 @@ def sp_sql
191210
RETURN(420) |
192211
end
193212

213+
def sp_error_sql
214+
%|CREATE PROCEDURE tinytds_TestPrintWithError
215+
AS
216+
PRINT 'hello'
217+
RAISERROR('Error following print', 16, 1)|
218+
end
219+
220+
def sp_several_prints_sql
221+
%|CREATE PROCEDURE tinytds_TestSeveralPrints
222+
AS
223+
PRINT 'hello 1'
224+
PRINT 'hello 2'
225+
PRINT 'hello 3'|
226+
end
227+
194228
def find_value(id, column, query_options={})
195229
query_options[:timezone] ||= :utc
196230
sql = "SELECT [#{column}] FROM [datatypes] WHERE [id] = #{id}"
@@ -214,4 +248,3 @@ def rollback_transaction(client)
214248

215249
end
216250
end
217-

0 commit comments

Comments
 (0)