Code review comment for lp:~myers-1/pyopenssl/npn

Thanks! A few comments:

  1. Where `global_next_protos_advertised_callback` invokes application code, `next_protos_advertised_callback`, it needs to check for an exception being raised and propagate it. It also needs to make sure the return value is a sequence and raise its own exception (likely a `TypeError`) if something else is returned. Notice the "XXX" near the end of `global_next_protos_advertised_callback`: that's the code doing the exception check. It doesn't do any good down there, after `ret` has been used. :)
  1. `global_next_proto_select_callback` needs to check the return value of `PyList_New`. It may return NULL to signal error.
  1. Is the reference counting for `conn->proto_selected` / `ret` correct? Does it need to be incref'd after `ret` is assigned to `conn->proto_selected`? I don't know who owns the reference returned by `PyEval_CallObject`.
  1. The `PyCallback_Check` in `ssl_Context_set_next_protos_advertised_callback` isn't necessary. Just try calling the object. If it fails, the exception handling code at the call site (still needs to be added :) will handle it.
  1. Same comment about `ssl_Context_set_next_proto_select_callback`.
  1. The unit tests should exercise as many of the error cases as possible.
  1. Avoid using assertEqual inside a callback in the unit test. There's no guarantee that the exception it raises will propagate to the test runner and be reported (and indeed, the current implementation will crash instead). Record the arguments and make assertions about them afterwards.

Thanks again! Looking forward to the next iteration.