Exception not raised correctly when thrift loaded with custom module_name

[JonnoFTW]

I have an issue when trying to raise and catch the exact exception from an included file. My thrift is:

#TestSpect.thrift
include "errors.thrift"
service TestService {
    void do_error() throws (1: errors.TestException e)
}

#errors.thrift
exception TestException {
    1: string message
}

The following pytest code succeeds:

def test_thrift_exception():
    import thriftpy2
    from thriftpy2.rpc import make_client, make_server
    from multiprocessing import Process
    from time import sleep
    test_thrift = thriftpy2.load("TestSpec.thrift", module_name="test_thrift")
    test_errors_thrift = thriftpy2.load("errors.thrift")

    class TestHandler:
        def do_error(self):
            raise test_errors_thrift.TestException("Error thrown!")

    def run_server():
        server = make_server(test_thrift.TestService, TestHandler())
        server.serve()
    p = Process(target=run_server,)
    p.start()
    sleep(0.5)
    try:
        client = make_client(test_thrift.TestService)

        with pytest.raises(test_errors_thrift.TestException) as e:
            client.do_error()
    finally:
        p.kill()

Changing test_errors_thrift to:

test_errors_thrift = thriftpy2.load("errors.thrift", module_name="test_errors_thrift")

Causes the test to fail. I suspect this is because test_thrift.TestService.do_error_result.thrift_spec is:

{1: (12, 'e', <class 'errors.TestException'>, False)}

But the error raised is a:

test_errors_thrift.TestException

So it can't actually raise the error correctly because it can't be put into the error result because:

isinstance(test_errors_thrift.TestException, errors.TestException)

returns False, per TProcessor.handle_exception from:

thriftpy2/thriftpy2/thrift.py

Lines 303 to 312 in a8b7873

	def handle_exception(self, e, result):
	for k in sorted(result.thrift_spec):
	if result.thrift_spec[k][1] == "success":
	continue
	_, exc_name, exc_cls, _ = result.thrift_spec[k]
	if isinstance(e, exc_cls):
	setattr(result, exc_name, e)
	return True
	return False

Would the correct fix here be to ensure that generated thrift_specs use the custom module name?

[ethe]

Thank you, I think it should be fixed.

[JonnoFTW]

@ethe after digging around in the code, it looks like these thrift_specs are being generated in _make_service here:

thriftpy2/thriftpy2/parser/parser.py

Lines 898 to 905 in a8b7873

	# result payload cls
	result_name = '%s_result' % func_name
	result_type = func[1]
	result_throws = func[4]
	result_oneway = func[0]
	result_cls = _make_struct(result_name, result_throws,
	_gen_init=False)
	setattr(result_cls, 'oneway', result_oneway)

Which comes from funcs which are a ply.yacc.YaccProduction which are generated by ply.

At this point I can only only see some kind of monkey-patching fix to replace the module instance after the service class is generated. Maybe there's some other way to do it but I'm not sure.