style: formalize exception policy on the data path.#4793
style: formalize exception policy on the data path.#4793mattklein123 merged 3 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Harvey Tuch <htuch@google.com>
|
@envoyproxy/maintainers |
| static) should throw meaningful `EnvoyException`s, the configuration | ||
| ingestion code will catch these. | ||
| - To indicate constructor failure. | ||
| * Do not use exceptions on the data path for general purpose error handling. |
There was a problem hiding this comment.
Not sure I really agree with this. Sometimes it makes things easier, sometimes not. What's the rationale for this change?
There was a problem hiding this comment.
A bit more context is here: #4707 (comment)
We need clearer guide line for using exception on data path.
There was a problem hiding this comment.
The main area in which exception handling is used currently on the data path is codecs/parsing which tend to have very deep call stacks, making return error handling onerous. I don't think we should prevent people from doing this since I think it's a reasonable pattern. I do agree we should make things more clear in what we expect (in general I think we expect people to not use exceptions when the error call stack is only a single call deep for example), but I would generally clarify that Envoy core will not catch any exceptions on the data path, so any not caught exception will cause the server to crash.
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
|
@mattklein123 discovered my secret attempt to outlaw exceptions in his absence :D J/K I've updated with feedback. |
4 participants
Signed-off-by: Harvey Tuch htuch@google.com