Fixed Template->render() exception handling #78

Merged
merged 1 commit into from Jul 9, 2015

Projects

None yet

2 participants

@elazar
Contributor
elazar commented Jul 9, 2015

Template->render() calls ob_get_clean() to get the result of rendering the template. According to the PHP manual page for this function, this is what it does, emphasis mine:

Gets the current buffer contents and delete current output buffer.

After this function is called, another block is executed to render the layout. Within this block, it's possible for a LogicException to be thrown if, for example, the layout template cannot be found. If this happens, there is a catch block to handle it.

There are two problems with this catch block:

  1. Because the aforementioned call to ob_get_clean() deleted the output buffer, the call to ob_end_clean() in the catch block has no buffer to delete and thus causes a notice to be emitted: "failed to delete buffer. No buffer to delete."
  2. Rather than the existing LogicException instance being re-thrown, a new one is being instantiated with the message of the original, causing the trace information associated with the original to be lost. This makes debugging difficult.

This PR applies these corrections:

  1. Wraps the ob_end_clean() call in a ob_get_length() check so that it is only called if needed, avoiding the previously emitted notice.
  2. Re-throws the existing LogicException instance so that the original trace information is preserved.
@elazar elazar Fixed Template->render() exception handling
If the template was found but the layout was not, a notice was
being emitted: "failed to delete buffer. No buffer to delete."

Additionally, within the branch to catch a LogicException, rather than the
existing LogicException instance being re-thrown, a new one was being
instantiated with the message of the original, losing trace information in the
process.
e686b8d
@reinink
Member
reinink commented Jul 9, 2015

Love it, thanks for working this out Matt!

@reinink reinink merged commit 2d8569e into thephpleague:master Jul 9, 2015

2 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@elazar
Contributor
elazar commented Jul 9, 2015

@reinink Ha, you're quick. I'd just tweeted this at you. :p Thanks!

@elazar elazar deleted the elazar:fix/template-render-exceptions branch Jul 9, 2015
@reinink
Member
reinink commented Jul 9, 2015

Haha, not always! Tagged.

@elazar
Contributor
elazar commented Jul 9, 2015

@reinink Ha! Was just about to ask if you'd tag a release. It's like you're in my head or something. ;) Thank you sir! :D

@localheinz localheinz referenced this pull request Aug 30, 2015
Closed

Fix: Do not clean more levels than needed #87

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment