Re: [RFC] [Discussion] Error backtraces v2
| From: | Tim Düsterhus | Date: | Mon, 09 Dec 2024 14:34:42 +0000 |
| Subject: | Re: [RFC] [Discussion] Error backtraces v2 | ||
| References: | 1 2 3 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-126120@lists.php.net to get a copy of this message | ||
Hi
Am 2024-12-06 02:08, schrieb Eric Norris:
I believe RFCs should be sufficiently precise so that one is able to implement the proposal reasonably unambiguously based on the RFC text alone. Given that you are concerned about the INI setting causing the RFC not to pass, it should be evident that this is such an important topic that it needs the RFC to be clear about what is proposed for voters to be able to vote on the full picture.The “php.ini Defaults” section should probably refer to the voting widget as well. The first voting widget would probably be better phrased as: "Add the 'error_backtrace_recording' INI as described?"I'm a little nervous about this change, because I was hoping to be a little more open-ended. For example, I'm not tied to the name 'error_backtrace_recording', so I wouldn't want someone to vote 'no' because they didn't like the setting name. Secondly, maybe we don't want the zend global (which I'll actually get to in a bit), and similarly I wouldn't want someone to vote 'no' because of that; I don't actually care about whether it's a global or not, I only care about getting backtraces for fatal errors.
Yes, I agree that having a stacktracke for fatal errors is a non-issue. For non-fatal errors the INI setting definitely is a problem. Unconditionally having a stacktrace for non-fatal errors I'm am not yet sure if this would be a problem or if it would be fine if it is a RFC-agreed-upon (breaking) change. Given that it's common to turn warnings and notices into Exceptions and thus to treat them as hard errors, I'd be inclined to say that the value-add of having backtraces for them is fairly small compared to the possible impact and that it makes sense not to include this in the RFC at all.Normally one would expect the "Unlocked" to be printed between "Done" and "After", but this is not the case, because the Exception in the global keeps a reference to $lock within its backtrace. This would be the same if the Exception would instead be aGreat example, thank you. Since you are stating this is problematic - and I don't necessarily disagree - does this make allowing backtraces as implemented for non-fatal errors a non-starter? Presumably having a backtrace for a fatal error doesn't matter since everything will soon be destroyed in the process of shutting down.trigger_error()that remains available viaerror_get_last(). In other words, we would have a INI setting that changes application behavior in subtle ways, which is a no-no.
Making breaking changes to the internal API is generally fine if there is a reason for making the change. Best regards Tim DüsterhusFor the previous RFC this apparently was a non-issue, because the stack was stored as a string. But for your RFC this is an issue, because based on the tests it's stored as an array.I would have preferred to make this a parameter to zend_error_cb, but I thought that changing the signature of that method might be too much of a backwards compatibility break (though this would only affect extensions). Making it a parameter would shorten the lifetime of the