Get rid of "THIS IS BAD" message for Exceptions that don't subclass MaharaException

Bug #1542126 reported by Aaron Wells
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Low
Aaron Wells

Bug Description

Since 2006, Mahara's exception handler has displayed this message if it handles an exception that is not a subclass of MaharaException:

[WAR] 1b (lib/errors.php:457) An exception was thrown of class Exception.
THIS IS BAD and should be changed to something extending MaharaException,
unless the exception is from a third party library.
Original trace follows

This may have been useful in 2006 when MaharaException was new, but since then we have pretty much eliminated cases in the core code where this would be useful, and it pretty much only comes up with false positives where the exception really is from a third-party library.

This message is also displayed if an instance of MaharaException (rather than a subclass of it) is caught. Again, maybe necessary in 2006 when we were supporting PHP4, but with PHP 5 we can just make MaharaException abstract, which will make it impossible to be instantiated directly.

As such, the message is unneccesary and just clutters up logs and adds an extra bit of panic and confusion to admins new to Mahara. So I think it'd be better to remove it, and simply print something like "[External library exception]:"

Aaron Wells (u-aaronw)
Changed in mahara:
milestone: none → 16.04.0
importance: Undecided → Low
status: New → In Progress
assignee: nobody → Aaron Wells (u-aaronw)
Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/6019

Revision history for this message
Aaron Wells (u-aaronw) wrote :

The easiest way to test this is with a test script that only throws an exception. Like this:

<?php
define('INTERNAL', 1);
define('PUBLIC', 1);
// define ('CLI', 1);
require('init.php');

throw new Exception('hey there');

Aaron Wells (u-aaronw)
Changed in mahara:
milestone: 16.04.0 → 16.10.0
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/6019
Committed: https://git.mahara.org/mahara/mahara/commit/287141c71d7682ccf163690b7fde71225b8cbd4f
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 287141c71d7682ccf163690b7fde71225b8cbd4f
Author: Aaron Wells <email address hidden>
Date: Fri Feb 5 16:43:53 2016 +1300

Bug 1542126: Cleaning up very old error handler code

This patch contains these three related changes:

1. Removes the "THIS IS BAD" message that was printed when 3rd-party
exception or an instance of MaharaException was thrown.

2. Removes the completely unnecessary "MaharaThrowable" interface.

3. Adds an comment explaining why MaharaException is not abstract

4. In the case of a smarty exception while printing an error, prints
the name and content of the smarty exception.

behatnotneeded: Covered by existing tests

Change-Id: I800a868bd187efb76ed37cca872ce262c2abbdb0

Robert Lyon (robertl-9)
Changed in mahara:
status: In Progress → Fix Committed
Robert Lyon (robertl-9)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.