FreeTDS Development Group

["James K. Lowden" <jklowden AT freetds.org>]

Error handling has been a longstanding weakness in FreeTDS, partly because
it's complex and partly because Sybase made it complex.  I'm speaking
mostly of dberrhandle().  It's hard enough to install a handler, a little
harder to return the right value, and very hard to interpret that value
correctly.  I'm pretty sure when Nick worked on it, he found that Sybase
didn't adhere to its own documentation. 

Intra-library
-------------

The first and simplest thing to do is to get rid of _dblib_client_msg(),
which needlessly sprinkles error text all over the library.  The text has
a fixed relationship to the number; all that's needed is a function that
accepts the msgno, looks up the text, and DRTR (calls the client's
handler, if any).  I wrote that function today, and I'm going to convert
all dblib.c to use it.  Chalk one up for consistency.  

Next up is no one's best friend: tds_client_msg().  The dblib call stack
looks like this:

        _dblib_client_msg()
        tds_client_msg()
        _dblib_handle_err_message()
        [client handler]

(I'm not sure about this leading underscore convention, either....)

tds_client_msg() is adding no value here.  What's needed is:

        _dblib_err(int msgno)
        _dblib_handle_err_message()
        [client handler]

So that's what we'll do.  :-)  

We also don't prevent recursion, and we should.  

Inter-library
-------------

Sometimes libtds has to call back to the client's installed handler.  We
need very clear semantics between libtds and the client API, because the
handler may be written according to several specifications, depending on
API and vendor.  We don't have them.  

Inbound, the error numbers that libtds emits are db-lib based, and the
parameters include some ct-lib (I think) state.  ct-lib's error numbering
scheme is much saner and thus inconsistent with db-lib.  My guess is ODBC
uses another set altogether.  My idea is to have libtds standardize on the
ct-lib numbers, and let the others map to that.  

Outbound, tds_client_msg() gets back a db-lib SUCCEED or FAIL, which
thankfully map to CS_SUCCEED and CS_FAIL, and their TDS bretheren.  But
woe to he who would like his handler to insist on re-trying a timeout:

        case INT_TIMEOUT:
                /* XXX do something clever */
                return SUCCEED;

If we're ever going to get timeouts to work according to spec, we're going
to have to let the handler have its say.  We need to synthesize the
meanings of all the handler conventions to convey clear information back
to libtds.  

(We've talked about this before.  The reason I'm interested now is that
we've done enough work on timeouts that error handling has become the weak
link in the chain.)  

We can keep tds_client_msg(), but we can't have it call
_dblib_handle_err_message(), because _dblib_handle_err_message(): 

1.  expects db-lib message numbers, where libtds should use libtds error
numbers.
2.  returns db-lib codes, because it's a db-lib function widely called
from withing db-lib.  It can't know it's been called from libtds.  

For errors that originate in libtds, we'll have tds_client_msg() call a
new db-lib function, maybe "_dblib_handle_tds_message()".  It will convert
the msgno, call _dblib_err(), and convert the return code to fit libtds's
specification.  

The good news is that all of this can be done within db-lib, initially.
Once the new functions have been written, we can convert the libtds error
message numbers and begin calling the new db-lib functions.  

I skipped a step, of course.  libtds has 15 calls like this:

tds_client_msg(tds->tds_ctx, tds, 20014, 9, 0, 0, "Login incorrect.");

It's wrong to map that text to that number, and it's wrong to send that
number to a ct-lib or ODBC handler.  What we need instead:

        tds_client_msg(tds, routine & layer & origin);

And make sure _dblib_handle_err_message() can deal.  Fortunately, 15 isn't
that many to do.  

I haven't looked at the ct-lib side.  It will be affected to, at least for
those 15 calls.  But, since we're walking libtds in ct-lib's direction,
the impact creator should be smaller.  

Lastly, I have a question about TDSMESSAGE::priv_msg_type.  It's written
to 5 times (once in mem.c, 4 in token.c).  It's never read.  Anyone have
any idea what it's meant to do?  Also, I'd like to s/msg_level/severity/
and s/msg_number/msgno/ and s/msg_state/state/ if there are no objections.
 Those are the terms used in the docs.  

typedef struct tds_message
{
        TDS_SMALLINT priv_msg_type;
        TDS_SMALLINT line_number;
        TDS_UINT msg_number;
        TDS_SMALLINT msg_state;
        TDS_SMALLINT msg_level;
        TDS_CHAR *server;
        TDS_CHAR *message;
        TDS_CHAR *proc_name;
        TDS_CHAR *sql_state;
} TDSMESSAGE;

Here's to better errors.  

--jkl