Author Topic: Better error handling (Read 658 times)

xavier · November 28, 2011, 04:52:31 am

Hi,

Several issues I think around error handling

1) some checks are done systematically (eg in civicrm_api3_phone_create, it verifies if the type id or location are correct).
Shouldn't it be tested only in case of error? ie. create first, it will fail at the DB level and then we can do the proper tests to get a clear error message

IMO, this slows down the API (eg it adds at least 2 extra selects, quite a lot when you want to do an insert of 10k phones)

2) some checks are missing
eg on the address api, the length of the strings (postal code, street address...) is not tested, and if too long, you get a rather opaque DB error.

Probably should be handled like 1, or add a mode "truncate if needed" ?

3) create error has an additional param to put other attributes in the result (on the top of the is_error and error_message). I've tried to add a error_code and other messages that are clearer and easier to parse. We should standardise and put them more often

What do you think?

X+

Eileen · November 28, 2011, 11:53:18 am

Hi,

As you know I implemented 1 type of error checking on the weekend - ie. IF the error is DB error: contstraint violation THEN it uses civicrm_api($entity,'getfields', $params) to find out which keys are foreign constraints & tests them. I wrote a couple of tests & removed the checks to see whether the contact id was valid from Contribution & Membership (at least one already had a test in place- yay).

It all seemed good & like a good compromise between performance & giving information.

As an aside the Import process 'sort of' uses the api (it calls internal functions directly in a way that bypasses some of apiv3 features & means it gets a reduced version of the v2 function). If it were to use it correctly (which I assume it will at some point) then it would be dependent on getting better error messages than 'DB error: contstraint violation' - I think increasingly people will be providing thin layers over the api so giving good error messages is worth doing.

xavier · November 28, 2011, 11:58:40 am

Hi,

While we are at promoting week-end work: if debug=1 it also gives the ['trace'] of the call, long, but full of info.

And if you have xdebug, it adds info about the speed and memory consumption.

Anyway, the error if string too long is less than clear, and I think we should follow what you did on foreign keys indeed.

X+

Eileen · November 28, 2011, 12:03:08 pm

NB - I think perhaps we should still add an option to do the intensive checking up front e.g

$params['options']['validation'] = 'max'

(That would also make sense in terms of how I've implemented it as I simply re-call the validate function with an extra param - at the moment it's an extra variable but I think setting an extra field makes more sense)

CiviCRM Community Forums (archive)

News:

Author Topic: Better error handling (Read 658 times)

xavier

Better error handling

Eileen

Re: Better error handling

xavier

Re: Better error handling

Eileen

Re: Better error handling