> it would be more logical to collect the trace in the ZEND_THROW Opcode > instead of in the create_handler of the Exception class
I've been pondering a means of doing this with relatively few BC issues. There are four use-cases right now: 1. Direct: throw new Exception() 2. Indirect: throw $factory->createException() 3. Re-throwing: throw $exception 4. Not throwing: $exception = new Exception() The first use-case (throw new) makes up the large majority, and would be unaffected by this change. The second use-case (indirect throw, factory) and third use-case (re-throwing) are both broken as-is - in either case, it isn't reporting the correct stack-trace now. This change corrects those cases. Even if that's a minor BC break, it corrects an error. By my analysis, that leaves only the 4th case (not throwing) as a potentially serious BC break. If somebody is presently constructing an Exception just as a means of extracting a stack-trace, this would no longer work. But there is a more direct way of doing that, which is debug_backtrace(), so likely the number of cases of this in the wild are few, and easy to correct, and likely with backwards-compatibility of the corrected code in all cases. I can't think of any other case where this change affects anything, but please correct me and point out any other case you can think of? As for Exception::getTrace(), I would propose the signature be changed to Exception::getTrace($index = 0) which would be backwards compatible in API terms - the default argument of 0 for $index would return the most recent stack trace, whereas an index of 1 would return the previous stack trace, e.g. after a re-throw. Exception::getTraceAsString() could either change in the same way, or render out all collected traces (which might be a bit much) or could simply render the most recently collected trace - I'm not sure which is best or most compatible. Exception::getLine() could either return the line-number of the first or last collected stack-trace - returning the first collected line-number is more compatible, consistent with "return new" and re-throwing, but returning the last line-number might be more correct, I'm not sure. Would a change like this require an RFC (and a vote) or is it arguably a bug? (I'm guessing this change is too substantial to be considered a "bug fix"?) On Mon, May 23, 2016 at 10:12 AM, Julien Pauli <jpa...@php.net> wrote: > On Sat, May 21, 2016 at 8:16 PM, Rasmus Schultz <ras...@mindplay.dk> wrote: >> Alright, so forget my comparison with other languages. >> >> My other points remain: >> >> Presently, "throw new" is treated as though it was one statement. >> That's not the case. We have deferred throws and factory methods for >> exceptions, and we have re-throws, so collecting the stack-trace at >> construction time doesn't work. >> >> The construction site would only be relevant if "throw new" was in >> deed a single statement. >> >> Recording the actual throw site is clearly the goal - the current >> implementation is betting on "throw" and "new" happening at the same >> site, which is merely circumstance. >> >> Ideally, an Exception should collect another stack trace for each >> successive throw, which would enable you to trace not only the >> original site, but the flow through any exception-handlers that might >> have re-thrown the same Exception. >> >> As is, there is no information collected on throw, and thereby no >> evidence or record of possible re-throws - on top of the fact that you >> may be collecting and looking at bogus stack-traces from factory >> methods or exception mappers. >> >> >> On Fri, May 20, 2016 at 11:06 AM, Rowan Collins <rowan.coll...@gmail.com> >> wrote: >>> On 20/05/2016 08:22, Niklas Keller wrote: >>>> >>>> 2016-05-20 4:13 GMT+02:00 Jesse Schalken <m...@jesseschalken.com>: >>>>> >>>>> The top frame is the construction (get_error) and the site of the throw >>>>> (do_throw) doesn't appear in the stack at all. >>>>> >>>> >>>> The comparison with JavaScript isn't a good one, since you can throw >>>> everything in JS. If they didn't provide the stack trace upon throw, you >>>> would not have a stack trace at all if you throw a plain string. >>>> >>> >>> >>> That explanation justifies completely the opposite behaviour to what Jesse >>> described. >>> >>> According to MDN [1] the "stack" property is completley unstandardised, and >>> some engines may indeed populate it on throw, but there's no hint on that >>> page that they'll attach it to anything not constructed as an Error. >>> >>> So it's not a great comparison for either side (note that it was originally >>> brought up by Rasmus as an example where it *does* come from the throw site) >>> because the language doesn't actually guarantee you a stack trace at all. >>> >>> [1] >>> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack >>> >>> Regards, >>> -- >>> Rowan Collins >>> [IMSoP] >>> >>> -- >>> PHP Internals - PHP Runtime Development Mailing List >>> To unsubscribe, visit: http://www.php.net/unsub.php >>> >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > Hi. > > I explained that in my article detailing Exceptions from internal , > http://jpauli.github.io/2015/04/09/exceptional-php.html > > I admit it would be more logical to collect the trace in the > ZEND_THROW Opcode instead of in the create_handler of the Exception > class. > > That would break backtraces, but we already broke them in PHP 7.0 > So we could think about it for a 8.0 ideally , or a 7.1 ; I'm not > sure. Anyway, that needs more debate ;-) > > > > Julien.Pauli -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
