[
https://issues.apache.org/jira/browse/THRIFT-6163?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-6163:
-------------------------------
Description:
h3. Problem
When a handler crashes, {{thrift_processor:handle_error/5}} puts the crash term
and the Erlang
stack trace into the {{TApplicationException}} it sends back to the caller.
{{exceptions_include_traces}} ships as {{true}} in
{{lib/erl/src/thrift.app.src}}, so that is
what an unmodified deployment does.
The trace names internal modules and the source paths of the machine that built
the release.
A caller that provokes a crash receives something along these lines:
{code}
{thrift_processor,handle_function,3,
[{file,"/build/lib/erl/src/thrift_processor.erl"},{line,128}]}
{code}
h3. It is not only the stack
{{handle_unknown_exception/5}} reaches {{handle_error/5}} for any exception the
IDL did not
declare, wrapping the thrown term into the error it forwards. So the term the
handler threw
goes to the caller as well. That is the application's own data rather than
Thrift's, and
nothing in Thrift can bound what a handler puts in it.
h3. Change
Default {{exceptions_include_traces}} to {{false}}. The switch stays: anyone
who wants traces
on the wire sets it and gets them unchanged, and the crash is still logged
locally in full
through {{error_logger}} either way. A release note goes under
{{lib/erl/README.md}}.
{{test/erl/src/thrift_test.app.src}} carried the same variable, but
{{handle_error/5}} reads it
out of the {{thrift}} application, not {{thrift_test}}, so nothing could ever
read that copy.
Removed rather than flipped.
h3. Tests
Three, over a real socket: the two failure routes into {{handle_error/5}} under
the shipped
configuration, and one that sets the variable and checks the trace still comes
back. The first
two fail against the previous default.
They load the {{thrift}} application first, and that matters -- with the code
merely on the code
path, {{application:get_env/2}} answers {{undefined}}, the fallback branch
runs, and a test that
skips the load passes against the old default.
_Drafted with AI assistance (Claude Opus 5); filed by Jens Geyer._
was:
h3. Problem
When a handler crashes, {{thrift_processor:handle_error/5}} puts the crash term
and the Erlang
stack trace into the {{TApplicationException}} it sends back to the caller.
{{exceptions_include_traces}} ships as {{true}} in
{{lib/erl/src/thrift.app.src}}, so that is
what an unmodified deployment does.
The trace names internal modules and the source paths of the machine that built
the release.
A caller that provokes a crash receives something along these lines:
{code}
{thrift_processor,handle_function,3,
[{file,"/build/lib/erl/src/thrift_processor.erl"},{line,128}]}
{code}
h3. It is not only the stack
{{handle_unknown_exception/5}} reaches {{handle_error/5}} for any exception the
IDL did not
declare, wrapping the thrown term into the error it forwards. So the term the
handler threw
goes to the caller as well. That is the application's own data rather than
Thrift's, and
nothing in Thrift can bound what a handler puts in it.
h3. Change
Default {{exceptions_include_traces}} to {{false}}. The switch stays: anyone
who wants traces
on the wire sets it and gets them unchanged, and the crash is still logged
locally in full
through {{error_logger}} either way. A release note goes under
{{lib/erl/README.md}}.
{{test/erl/src/thrift_test.app.src}} carried the same variable, but
{{handle_error/5}} reads it
out of the {{thrift}} application, not {{thrift_test}}, so nothing could ever
read that copy.
Removed rather than flipped.
h3. Tests
Three, over a real socket: the two failure routes into {{handle_error/5}} under
the shipped
configuration, and one that sets the variable and checks the trace still comes
back. The first
two fail against the previous default.
They load the {{thrift}} application first, and that matters -- with the code
merely on the code
path, {{application:get_env/2}} answers {{undefined}}, the fallback branch
runs, and a test that
skips the load passes against the old default.
> Erlang: do not send handler crash detail to the caller by default
> -----------------------------------------------------------------
>
> Key: THRIFT-6163
> URL: https://issues.apache.org/jira/browse/THRIFT-6163
> Project: Thrift
> Issue Type: Bug
> Components: Erlang - Library
> Reporter: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h3. Problem
> When a handler crashes, {{thrift_processor:handle_error/5}} puts the crash
> term and the Erlang
> stack trace into the {{TApplicationException}} it sends back to the caller.
> {{exceptions_include_traces}} ships as {{true}} in
> {{lib/erl/src/thrift.app.src}}, so that is
> what an unmodified deployment does.
> The trace names internal modules and the source paths of the machine that
> built the release.
> A caller that provokes a crash receives something along these lines:
> {code}
> {thrift_processor,handle_function,3,
> [{file,"/build/lib/erl/src/thrift_processor.erl"},{line,128}]}
> {code}
> h3. It is not only the stack
> {{handle_unknown_exception/5}} reaches {{handle_error/5}} for any exception
> the IDL did not
> declare, wrapping the thrown term into the error it forwards. So the term the
> handler threw
> goes to the caller as well. That is the application's own data rather than
> Thrift's, and
> nothing in Thrift can bound what a handler puts in it.
> h3. Change
> Default {{exceptions_include_traces}} to {{false}}. The switch stays: anyone
> who wants traces
> on the wire sets it and gets them unchanged, and the crash is still logged
> locally in full
> through {{error_logger}} either way. A release note goes under
> {{lib/erl/README.md}}.
> {{test/erl/src/thrift_test.app.src}} carried the same variable, but
> {{handle_error/5}} reads it
> out of the {{thrift}} application, not {{thrift_test}}, so nothing could ever
> read that copy.
> Removed rather than flipped.
> h3. Tests
> Three, over a real socket: the two failure routes into {{handle_error/5}}
> under the shipped
> configuration, and one that sets the variable and checks the trace still
> comes back. The first
> two fail against the previous default.
> They load the {{thrift}} application first, and that matters -- with the code
> merely on the code
> path, {{application:get_env/2}} answers {{undefined}}, the fallback branch
> runs, and a test that
> skips the load passes against the old default.
> _Drafted with AI assistance (Claude Opus 5); filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
