Re: [reSIProcate] replacing assertions
On 12/12/14 16:02, Scott Godin wrote:
> I believe there are two main types of assertions in resip:
> 1. Assertions that really signify a bug in the resip stack. These are
> mainly in the stack and rutil projects.
> 2. Assertions caused by miss-use of the public API's. Most of these
> are in the DUM API's. ie: attempting things in an Invite session when
> in the wrong state.
>
> I think the main issue is with the assertions that fall into #2. I
> would like to see a sweep of the DUM code to convert the ones that make
> sense from assertions to throws. I've already done some this for the
> provideOffer and provideAnswer API's.
>
> Note: In most cases the assertions that fall into #1 will not benefit
> from a conversion to throw. These are typically triggered in internal
> bug conditions only where throwing or continuing through the assertion
> would not fix the damage. It's also quite likely that many code
> locations that do #1 assertions would not catch exceptions anyway.
>
I think there are two potential benefits of not using assert:
- people can't hide these things with NDEBUG, runtime_error may still be
appropriate for things in the first category, even if there is no way to
fix them at runtime
- if people are embedding the stack within a bigger application then
they may catch the exception at a higher level and shut down other parts
of the application more gracefully
Looking at the TLS code last week I noticed there would be an assertion
if I put a bad cipher string in repro.config. This is probably a good
example of the type of thing that needs to be reported more cleanly for
a sysadmin to work productively with repro.
Some more things to consider:
- the definition of resip_assert could also include a log statement
and/or it could be defined to log directly to syslog for those on Linux.
- we could create some hook in the travis-ci build to detect any new
commits that include an assert and complain about them (unless they are
in a test directory)