uber / tchannel

network multiplexing and framing protocol for RPC
MIT License
1.15k stars 129 forks source link

Don't retry on unexpected Exception #1392

Open jc-fireball opened 8 years ago

jc-fireball commented 8 years ago

This is created based on some feedback from our existing users.

The unexpected exception error happens because tchannel captures uncaught exceptions from endpoint handler on the side and then send unexpected error frame back to the client. Right now by default client will retry on unexpected error frame.

However most of the case if the unexpected error happens, probably the user's handler code has some bug causing this. Even we do retry and redirect to other server instance. The instance most likely has the same code deployed. It doesn't make sense to retry on this. It just waste time.

So we may change the protocol to not retry or only retry on certain flag.

Raynos commented 8 years ago

Let's improve protocol retry flags.

Instead of nt lets have unt.

In that PR we can talk about recommended defaults; probably un by default for back compat.

jcorbin commented 8 years ago

I could agree with adding a "u" flag to opt in for "retry on unexpected error", but it should not default on; retrying on unexpected error seems wrong to me.

Raynos commented 8 years ago

Let's also sync up with @mranney about the default.