Renderpartial() And Exceptions

[Keith (Kburton)]

[b]EDIT: I’m moving this into the Bug Discussions forum for further discussion.

Operating System: Ubuntu 13.10

Web Server: Apache 2.4.6, PHP 5.5.3

Browser: Chrome (should be irrelevant)

Yii Version: 1.1.14-dev[/b]

I’ve encountered what seems to be a bug when an Exception is thrown from a view during a renderPartial() call. Here’s a minimal set up which exhibits the problem:

The controller:

class TestController extends Controller

{

	public function actionTest()

	{

		$success = true;

		$html = '';

		

		try

		{

			$html = $this->renderPartial('test', array(), true);

		}

		catch (Exception $ex)

		{

			$success = false;

		}

		

		echo CJSON::encode(array('success'=>$success, 'html'=>$html));

	}

}

The view:

Some HTML content.


<? throw new Exception; ?>

The output:

Some HTML content.


{"success":false,"html":""}

That’s a simplified example of what I’m doing during an AJAX call. An exception thrown in the view is causing PHP to output the preceding content straight to the browser, which invalidates the JSON response that I should be outputting.

I can resolve the problem by calling ob_clean() in the catch clause, but I wonder whether Yii itself should catch the exception and re-throw it after clearing the buffer.

Any thoughts?

[twisted1919 (Serban Cristian)]

This is the expected behavior, nothing is wrong.

You should never throw errors in your views, if you are doing it, you are doing something wrong.

[tgr (Trond)]

This is the expected behavior, nothing is wrong.

You should never throw errors in your views, if you are doing it, you are doing something wrong.

Interesting. Can you enlarge on that?

[Keith (Kburton)]

This is the expected behavior, nothing is wrong.

You should never throw errors in your views, if you are doing it, you are doing something wrong.

The only practical way to avoid it in my case would be to build the HTML output in the controller, which defeats the purpose of MVC. I’m using the view to build a table (basically a report) which calls methods of the model to determine the content and formats of each cell. Given that a report could have thousands of lines, it doesn’t make sense to iterate over all of the data once to build up an array of values and formats, just to pass that into the view and iterate over it again to output it.

I know that it’s bad practice to have logic in the view, so the logic is encapsulated in the models and in an iterator class that I created to manage row groupings. The model can throw exceptions, though, if it encounters unexpected data, and this should be handled gracefully rather than sending the part of the view already processed straight to the browser.

I’d be interested to hear more opinions on this. If we accept that exceptions should never be thrown while a view is being rendered, what’s the best way to handle this type of scenario?

[twisted1919 (Serban Cristian)]

@trond - view files are meant only to take variables, iterate and/or echo them, plus conditionals of course, but nothing more.

Of course if you are looping through an array of models and you call simple getters won’t be a problem, but if the getters are relations that in turn are lazy loaded and will produce queries in views, then it is not okay.

@keith - how about having a component dedicated to this thing?

For example, in my mailwizz app, i have an option to export basic stats about a campaigns, a lot of things are going on inside that component (a behavior actually), ~600 lines of code with getters, setters and process methods. But because it is a behavior that i can attach to the "Campaign" model, i can use it when users clicks to export reports from the web interface, i also use it in console app to send an email with stats when the campaign finish sending, but i also use it in the campaign overview page to display various stats about a campaign.

This not only that allows me to reuse code in multiple places, but if in future i need to change something, i don’t need to do it in 10 places.

I am almost sure that such alternative would work for you too, even if you don’t depend on a single model to generate the stats, you could use DI into a single component and process accordingly inside it.

[tgr (Trond)]

Makes sense. I agree.

[Keith (Kburton)]

I’m still not sure about this. I feel that the whole point of a view is to help wrap your data in the relevant HTML tags for display. Here’s the part of the view generating the table:

	<table>

		<thead>

			<tr>

				<? foreach ($reportSheet->columns as $column): ?>

					<th><?= CHtml::encode($column->Name); ?></th>

				<? endforeach; ?>

			</tr>

		</thead>

		<tbody>

		

		<? $startTime = microtime(true); ?>

		

		<? foreach ($dataIterator as $row): ?>

			<? if ($dataIterator->getHasSubtotals()

					&& $dataIterator->groupHasChanged()): ?>

				<tr class="subtotals">

					<? foreach ($reportSheet->columns as $column): ?>

						<td class="<?= $column->getCellCssClass(); ?>">

							<?= $column->getSubtotalHtmlValue(); ?>

						</td>

					<? endforeach; ?>

				</tr>

			<? endif; ?>

			

			<tr>

				<? foreach ($reportSheet->columns as $column): ?>

					<td class="<?= $column->getCellCssClass(); ?>">

						<?= $dataIterator->getHtmlColumnValue($column); ?>

					</td>

				<? endforeach; ?>

			</tr>

		<? endforeach; ?>

		

		<? if ($dataIterator->getHasSubtotals()): ?>

			<tr class="subtotals">

				<? foreach ($reportSheet->columns as $column): ?>

					<td class="<?= $column->getCellCssClass(); ?>">

						<?= $column->getSubtotalHtmlValue(); ?>

					</td>

				<? endforeach; ?>

			</tr>

		<? endif; ?>


		<? if ($dataIterator->getHasTotals()): ?>

			<tr class="totals">

				<? foreach ($reportSheet->columns as $column): ?>

					<td class="<?= $column->getCellCssClass(); ?>">

						<?= $column->getTotalHtmlValue(); ?>

					</td>

				<? endforeach; ?>

			</tr>

		<? endif; ?>


		</tbody>

	</table>

To me, that much HTML content belongs in a view. The view itself contains very little logic and all of the calculations are done in the iterator and the model. Under certain circumstances, the model may throw an exception, which I should be able to catch in the controller and provide an error message to the user, rather than having essentially junk sent to the browser.

I do appreciate your point, twisted1919, but I still think this should be within the remit of the view. Even if I create a separate component, this HTML code is going to have to go somewhere.

[twisted1919 (Serban Cristian)]

Ok, i see your point and there might be a way out from this.

What if you wrap your html and logic for that chunk of view into a widget and you call that widget in controller in a try/catch block and make it return the output instead of render it? Then you would pass the output to a variable in the controller and just echo it in the view.

Wouldn’t that work for you ?

In case it doesn’t, i think it’s fine to take this issue to the github and have the core developers look into it.

[Keith (Kburton)]

I’m sure I could find a work-around to avoid using renderPartial(), but I’m not convinced that this isn’t a bug.

I’ll move the thread into Bug Discussions to see what the dev team have to say.