BIND 10 #2903: need more asio error handling in asiodns

[BIND 10 Development]

#2903: need more asio error handling in asiodns
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  b10-auth      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130423
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 I'm not strictly against unrelated cleanups, but this amount seems like
 50% of the branch. If already written, I don't think it is worth throwing
 it out, or trying to rebuild the history for two branches, but the history
 would be cleaner if the works were separated. So, include it now, but
 let's be more careful about the amount of unrelated work in future.
 Anyway, I think there might be something wrong with out work process, if
 we can't push these cleanups in some fast way without cluttering other
 branches.

 Also, a test in distcheck is failing for me. I don't know if it might be
 related, though. Should I try to find the cause of it?
 {{{
 ...F.......
 ======================================================================
 FAIL: test_bad_data (__main__.TestUserMgr)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File
 "/var/tmp/bind10/bind10-2/bind10-20130221/_build/../src/bin/usermgr/tests/b10
 -cmdctl-usermgr_test.py", line 477, in test_bad_data
     'add', 'user1', 'pass1'
   File
 "/var/tmp/bind10/bind10-2/bind10-20130221/_build/../src/bin/usermgr/tests/b10
 -cmdctl-usermgr_test.py", line 141, in run_check
     self.assertEqual(expected_stdout, stdout.decode())
 AssertionError: 'Using accounts file: test_users.csv\nError parsing csv
 file: newline inside str [truncated]... != 'Using accounts file:
 test_users.csv\n'
   Using accounts file: test_users.csv
 - Error parsing csv file: newline inside string
 }}}

 Can you explain in which cases this can throw? Allocation errors?
 Something else? If so, would it be some internal asio error?
 {{{#!diff
 -    io_.post(*this);
 +    io_.post(*this);  // this can throw, but can be considered fatal.
 }}}

 Many `YIELD return` statements are replaced just by `return`? Is it safe?
 Was it YIELD really unneeded? Any idea why was it there before?

 Also, in many error cases, should we log? At least DEBUG?

 What was the use of `checkin_callback`? Is it used anywhere? Why the other
 server classes need it and the sync one does not? Should there be more
 explanation in the doxygen, than just „unused“?

 Are these really unused? If so, why not remove them completely? Or do I
 understand the comment wrong?
 {{{#!diff
 -    // Objects to hold the query message and the answer
 +    // Objects to hold the query message and the answer: these are not
 used
      isc::dns::MessagePtr query_, answer_;
 }}}

 In the last commit, you use object variable instead of local one. But, as
 all the local occurrences in other servers are `ec`, should this one be
 `ec_` instead of `ecode_`, for consistency?

-- 
Ticket URL: <http://bind10.isc.org/ticket/2903#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development