"C. Michael Pilato" <cmpilato@collab.net> writes:
> "C. Michael Pilato" <cmpilato@collab.net> writes:
>
> > It would appear that in neon 0.24.3, the error is never left NULL, but
> > is instead initialized to "Unknown error". There must, then, be some
> > other way to check for an errorful state. Now... where is that
> > booger...
>
> A-ha! We should be calling ne_xml_valid(), and only if the XML was
> invalid should we try to fetch the error. Testing a patch now.
Okay, well, this turned out to be trickier than I thought.
Our XML parser callbacks tend to, on errorful conditions, set a
descriptive svn_error_t in their batons and then return XML_INVALID.
Now, in Neon 0.24.2, it seems that just returning XML_INVALID causes
neon to abort the parsing of the request response, but it wouldn't
touch its own internal error message field (accessed via
ne_xml_get_error()), choosing to leave it NULL as initialized. This
meant that svn_ra_dav__parsed_request (which just checked
ne_xml_get_error()) would *not* return an error, callers of that
function would not immediately return errorfully, and they would then
check their own baton's for errors (which they would of course find
and return). Wah-lah! You get a descriptive error message.
In neon 0.24.3, the neon error string is now never NULL or empty, so
we can't really use that as our "did this parse fail" check. So we
have to use ne_xml_valid() to see if the parse failed, and if it did,
we return the neon error. Of course, most of the times the XML isn't
valid is because some Subversion error occurred and the callbacks
returned XML_INVALID. This means that neon knows the XML was invalid,
but still has its error message set to "Unknown error" (it only
changes its own error messsage when some internal error occurs).
This, in turn, means that svn_ra_dav__parsed_request() will return a
parser error that has virtually no useful information (I mean,
"Unknown error" gets to be less than unique across the many places
errors could happen), which callers will immediately return instead of
noticing that some really useful Subversion error also exists in their
baton. Boo-hoo! You get a useless error message.
Three solutions:
(1) Change the XML parser callbacks (start_element et al) to,
instead of just returning XML_INVALID, also call
ne_xml_set_error(). Of course, this neon function takes a
string, which means we'd have to have code that flatters out
the messages in the svn_error_t * chain to a single 'const char
*'. Oh, but neon doesn't tell you that it will truncate the
error message you transmit through this function to 2048
characters. :-(
(2) Make svn_ra_dav__parsed_request() treat the neon error with the
string "Unknown error" the same way we currently treat NULL or
the empty string. This just seems wrong.
(3) Make callers of svn_ra_dav__parsed_request() first return their
baton's svn_error_t first, then fallback to the error returned
from svn_ra_dav__parsed_request().
For now, I'm going with solution 3.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 8 09:27:02 2003