search

erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

Pass stacktraces on to handle_crash #459

Closed jesperes closed 1 year ago

jesperes commented 1 year ago

This allows the user-defined crash function crashmsg/3 to get the actual stacktrace term.

vinoski commented 1 year ago

Thanks, I'll take a look shortly.

avtobiff commented 1 year ago

You need to make handle_crash/2 take the extra St argument and also pass it along to a user defined crashmsg/4 instead of the existing crashmsg/3.

Furthermore, you need to modify all calls to handle_crash/2 to also pass the stack to handle_crash/3.

jesperes commented 1 year ago

You need to make handle_crash/2 take the extra St argument and also pass it along to a user defined crashmsg/4 instead of the existing crashmsg/3.

Furthermore, you need to modify all calls to handle_crash/2 to also pass the stack to handle_crash/3.

Not sure I follow. handle_crash/3 already calls into handle_crash/2, packing the L and St arguments into a single argument. There are some places where handle_crash/2 is called directly with a string, and in those cases that string is passed directly to the user-defined crashmsg/4. The only thing this change does is to pass St as a stacktrace argument into handle_crash/2 and further on into the user-defined crashmsg/4.

(We have been running with this change in production for several months now, and it works nicely.)

avtobiff commented 1 year ago

This PR doesn't compile


$ make debug
(...)
  ERLC     ../ebin/yaws_server.beam
yaws_server.erl:3458: function handle_crash/3 undefined
yaws_server.erl:3466: function handle_crash/3 undefined
jesperes commented 1 year ago

This was an incomplete application of an internal patch of ours, and it is indeed not backwards compatible with existing crashmsg/3 implementations.

We have handle_crash/3 defined in yaws_server.erl as 

%% Allow to pass the log string as well as a stacktrace.
%% This patch has to be supported by the errormod, otherwise it might crash.
handle_crash(ARG, L, ST) ->
  handle_crash(ARG, {L, ST}).

handle_crash/2 then passes {L, ST} on as the third arg to crashmsg/3, but this is of course not backwards compatible with existing crashmsg/3 implementations. It would probably be preferrable to invoke crashmsg/4 (if it exists) with an extra St argument, otherwise just discard the St argument and fallback to crashmsg/3.

I'll close this PR and get back with a better implementation. Sorry about the noise.

avtobiff commented 1 year ago

I think it's a good enhancement that you want to add. Awaiting your PR!