Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Exception::getLine() »

Exception::getLine()

php.internals

Rasmus Schultz

10 years ago
Does this work as intended? function create_exception() { return new RuntimeException(); // line 2 } try { throw create_exception(); // line 6 } catch (Exception $e) { var_dump($e); // => 2 } I would have expected Exception::getLine() to return 6 in this case - the line where the exception was thrown. I know that I can dig in via e.g. Exception::getStackTrace()[0]["line"] and pull the relevant line-number myself, but I'm wondering why the line-number 2 is relevant, important or interesting at all? It's not uncommon to have e.g. static factory methods inside the exception itself, which would cause the exception instance to report the line-number of the new-statement inside the factory constructor, which has no particular importance or relevance for any purpose, as far as I can figure. Apparently this has been reported as a bug before: https://bugs.php.net/bug.php?id=53882 There was talk of a fix in the comments, but it looks like this issue went stale and was closed without any further explanation? The documentation is also a little ambiguous on this point: http://php.net/manual/en/exception.getline.php At the top of the page, it says "Gets the line in which the exception occurred", which isn't correct. (The rest of the page correctly says "the line number where the exception was created") Would it adversely affect anything to fix this? It's technically a BC break, I guess. Perhaps adding a dedicated getLineThrown() method would be better with regards to BC?

Tim Düsterhus

10 years ago
On 19.05.2016 13:14, Rasmus Schultz wrote:
> I know that I can dig in via e.g. > Exception::getStackTrace()[0]["line"] and pull the relevant > line-number myself, but I'm wondering why the line-number 2 is > relevant, important or interesting at all?
If you are rethrowing exceptions inside the catch bloc: for ($code = 0; $code < 2; $code++) { try { throw new Exception('Message '.$code, $code); } catch (\Exception $e) { if ($e->getCode() > 0) { throw $e; } } } Inside the catch bloc I check whether the Exception code is one that can be handled at that place (code = 0), if not I rethrow the exception. If the line property would refer to the place where the exception was thrown I'd lose information in that case. Tim

Sebastian Bergmann

10 years ago
Am 19.05.2016 um 13:14 schrieb Rasmus Schultz:
> Does this work as intended?
According to https://bugs.php.net/bug.php?id=64910 it does :-(

Niklas Keller

10 years ago
Sebastian Bergmann <sebastian@php.net> schrieb am Do., 19. Mai 2016 14:12:
> Am 19.05.2016 um 13:14 schrieb Rasmus Schultz: > > Does this work as intended? > > According to https://bugs.php.net/bug.php?id=64910 it does :-( > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
Resending to complete list: Yes, this is intended. Exceptions in PHP are always populated upon creation. Please note the already mentioned issues with rethrowing and also, that some exceptions might not get thrown at all, e.g. because of the use of promises.

Rasmus Schultz

10 years ago
> Exceptions in PHP are always populated upon creation
Wow. Well, I've always wondered why the throw site wasn't in the stack trace - this explains that. This is inconsistent with at least JavaScript and C#, where the stack trace is populated at the throw site. (Probably others?) That doesn't really make any sense to me, for a number of reasons. First, who cares where the Exception was constructed? That has no relevance, and it's often a factory method or in some cases a more complex facility, such as the one in Doctrine DBAL that maps PDO error info to an Exception. You will miss the actual throw site - unless it happens to be the same site as where the constructor was invoked, but there's no guarantee of that. When exceptions get re-thrown, there is no trace of the actual site from where the unhandled Exception was thrown - you're lugging around a stack trace that doesn't actually have the current stack. Technically, every throw is a new exception "flow" - even if you're recycling the Exception instance, it's the throw statement that starts the unique stack unwind from the throw site; it's where the action happens. What's important then, is the throw statement, not the object, since the site where the object happens to get created has no bearing on the stack at the time when the exception gets thrown or re-thrown. In other words, throw new Exception() only happens to work most of the time because the throw and new statements happen to be issued at the same site. This is wonky. Can we fix it? On Thu, May 19, 2016 at 3:05 PM, Niklas Keller <me@kelunik.com> wrote:

Rowan Collins

10 years ago
On 19/05/2016 19:35, Rasmus Schultz wrote:
> Technically, every throw is a new exception "flow" - even if you're > recycling the Exception instance, it's the throw statement that >
starts the unique stack unwind from the throw site; it's where the > action happens. That's one interpretation, but it doesn't really hold up in all cases. Consider a catch statement that needs to filter more granularly than the class name; since you already mentioned PDO, I'll make an example with that: catch ( PDOException $e ) { if ( substr($e->getCode(), 0, 2) === '08' ) { $this->reconnect(); } else { throw $e; } } Of what value to a subsequent catch statement is the trace of that throw statement? And why does that "start a new exception flow", but if PDO threw different sub-classes, you could let one flow through unmodified by tightening the catch statement? Regards,
-- Rowan Collins [IMSoP]

Ryan Pallas

10 years ago
On Thu, May 19, 2016 at 1:47 PM, Rowan Collins <rowan.collins@gmail.com> wrote:
> On 19/05/2016 19:35, Rasmus Schultz wrote: > >> Technically, every throw is a new exception "flow" - even if you're > >> recycling the Exception instance, it's the throw statement that > >> > starts the unique stack unwind from the throw site; it's where the > > action happens. > > That's one interpretation, but it doesn't really hold up in all cases. > Consider a catch statement that needs to filter more granularly than the > class name; since you already mentioned PDO, I'll make an example with that: > > catch ( PDOException $e ) { > if ( substr($e->getCode(), 0, 2) === '08' ) { > $this->reconnect(); > } else { > throw $e; > } > } > > Of what value to a subsequent catch statement is the trace of that throw > statement? And why does that "start a new exception flow", but if PDO threw > different sub-classes, you could let one flow through unmodified by > tightening the catch statement? >
True, but if you instead of throwing the same exception, threw a new one, you could capture both stacks. But you can never get the stack from the throw point of something created elsewhere. catch ( PDOException $e ) { if ( substr($e->getCode(), 0, 2) === '08' ) { $this->reconnect(); } else { throw new PDOException($e->getMessage(), $e->getCode(), $e); } }

Rasmus Schultz

10 years ago
> True, but if you instead of throwing the same exception, threw a new one, you could capture both stacks. But you can never get the stack from the throw point of something created elsewhere.
Precisely. I think there are good reasons why other languages collect the stack-trace on throw. Collecting the stack trace at construction would be correct if "throw new" was literally one statement - if the only time you could throw was at creation. 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. Of course, fixing this would require a BC break, and nobody wants to even entertain the thought of those :-( On Thu, May 19, 2016 at 10:30 PM, Ryan Pallas <derokorian@gmail.com> wrote:

Rowan Collins

10 years ago
On 19/05/2016 21:30, Ryan Pallas wrote:
> > Of what value to a subsequent catch statement is the trace of that > throw statement? And why does that "start a new exception flow", > but if PDO threw different sub-classes, you could let one flow > through unmodified by tightening the catch statement? > > > True, but if you instead of throwing the same exception, threw a new > one, you could capture both stacks.
The same could be said of the current behaviour:
> throw new PDOException($e->getMessage(), $e->getCode(), $e);
Since this contains both "throw" and "new", it's guaranteed a new stack trace in either convention. So the question remains which is the *more likely* desired behaviour if you *don't* re-wrap it, and I honestly don't know the answer.
-- Rowan Collins [IMSoP]

Jesse Schalken

10 years ago
On Fri, May 20, 2016 at 4:35 AM, Rasmus Schultz <rasmus@mindplay.dk> wrote:
> This is inconsistent with at least JavaScript and C#, where the stack > trace is populated at the throw site. (Probably others?) >
I'm not sure about C#, but in JavaScript (Node.js): function get_error() { return new Error('my error'); } function do_throw(e) { throw e; } try { do_throw(get_error()); } catch (e) { console.log(e.stack); } results in: Error: my error at get_error (/home/jesse/src/test.js:2:12) at Object.<anonymous> (/home/jesse/src/test.js:10:14) at Module._compile (module.js:413:34) at Object.Module._extensions..js (module.js:422:10) at Module.load (module.js:357:32) at Function.Module._load (module.js:314:12) at Function.Module.runMain (module.js:447:10) at startup (node.js:146:18) at node.js:404:3 The top frame is the construction (get_error) and the site of the throw (do_throw) doesn't appear in the stack at all.

Niklas Keller

10 years ago
2016-05-20 4:13 GMT+02:00 Jesse Schalken <me@jesseschalken.com>:
> > On Fri, May 20, 2016 at 4:35 AM, Rasmus Schultz <rasmus@mindplay.dk> > wrote: > >> This is inconsistent with at least JavaScript and C#, where the stack >> trace is populated at the throw site. (Probably others?) >> > > I'm not sure about C#, but in JavaScript (Node.js): > > function get_error() { > return new Error('my error'); > } > > function do_throw(e) { > throw e; > } > > try { > do_throw(get_error()); > } catch (e) { > console.log(e.stack); > } > > > results in: > > Error: my error > at get_error (/home/jesse/src/test.js:2:12) > at Object.<anonymous> (/home/jesse/src/test.js:10:14) > at Module._compile (module.js:413:34) > at Object.Module._extensions..js (module.js:422:10) > at Module.load (module.js:357:32) > at Function.Module._load (module.js:314:12) > at Function.Module.runMain (module.js:447:10) > at startup (node.js:146:18) > at node.js:404:3 > > > 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.

Rowan Collins

10 years ago
On 20/05/2016 08:22, Niklas Keller wrote:
> 2016-05-20 4:13 GMT+02:00 Jesse Schalken <me@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]

Rasmus Schultz

10 years ago
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.collins@gmail.com> wrote:

Julien Pauli

10 years ago
On Sat, May 21, 2016 at 8:16 PM, Rasmus Schultz <rasmus@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.collins@gmail.com> wrote: >> On 20/05/2016 08:22, Niklas Keller wrote: >>> >>> 2016-05-20 4:13 GMT+02:00 Jesse Schalken <me@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

Rasmus Schultz

10 years ago
> 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 <jpauli@php.net> wrote: