Elkarte icon indicating copy to clipboard operation
Elkarte copied to clipboard

What the plan for class Elk_Exception

Open Spuds opened this issue 8 years ago • 10 comments

This class is used in calendar and one in notifications ... looks to either log an message or fatal error. Just trying to understand what the plan is here vs existing or new methods of class Errors ?

Spuds avatar Dec 17 '15 22:12 Spuds

I'm sorry I completely missed that!! :blush:

I throw it in in 2014. At the time I think (sorry, my mind can't remember things so far in the past LOL) I put it in as a wrapper to fatal_lang_error, since I was playing with OpenImporter and started using exceptions there.

TBH I didn't put that much thinking in it, or its possible uses, just seemed interesting and kind of nice.

emanuele45 avatar Jan 27 '16 00:01 emanuele45

I was just trying to understand its usage vs the static error.class thing we also have ... not sure if they could or even should be combined.

Spuds avatar Jan 27 '16 01:01 Spuds

After almost one year, I feel (no real-life data) the two could live together, in some places throw an exception seems "better" than just die with a fatal error (at least you can catch the exception if needed and deal with it).


what a pain, 3 tabs now to post a comment... :sob:

emanuele45 avatar Oct 04 '16 12:10 emanuele45

nods ... the errors class is what gets set as our set_exception_handler I think my original question was can that be improved or simply replaced by the elk_exception such that the two could be combined as it just seemed like some duplication.

Spuds avatar Oct 04 '16 13:10 Spuds

I guess they could... I guess. lol

emanuele45 avatar Oct 04 '16 13:10 emanuele45

Something is not right anyway ...

  • Setup Calendar to allow events without a linked post.
  • Selected a day on the calendar to make an event
  • Made sure link to post was not checked and left the event title empty on the event form
  • Submitted the form
  • This triggers throw new Elk_Exception('event_title_missing', false);

Result was then a white screen with

Notice: No event title was entered.
Calendar_Event->validate() in /sources/subs/CalendarEvent.class.php at line 108
Calendar_Controller->action_save() in /sources/controllers/Calendar.controller.php at line 202
Calendar_Controller->action_post() in /sources/SiteDispatcher.class.php at line 303
Site_Dispatcher->dispatch() in /index.php at line 261
elk_main() in /index.php at line 191

Is this the expected behavior, or should it return to the form with the error message?

Spuds avatar Oct 04 '16 14:10 Spuds

You broke it!! :stuck_out_tongue_closed_eyes: https://github.com/elkarte/Elkarte/commit/92253dbc4e05bd115caa260191d93d11f91c4dfe#diff-4b7a3d18666f61ba05776dfac1d2468cL263

With the previous code, it was terminating in a fatal error. Yes, slightly redundant, the point was just not let the "subs"/"module" kill the process, but give the controller that power (sort of not use superglobals in subs).

emanuele45 avatar Oct 04 '16 15:10 emanuele45

That was 18months ago so It does not count ! ... Anyway its still dying with a fatal error, just inside of the error.class exception_handler function, without debug you would just get a white screen with Notice: No event title was entered.

Either way why would forgetting to enter a subject be fatal 😇 I guess notice errors could be routed to the "pretty" fatal error screen while warn and error get the hard boot ... or maybe just error?

Spuds avatar Oct 04 '16 21:10 Spuds

Heck! :P Try these changes: d2064f1f4a43b996fcc61bc26b790df18592a663

Elk_Exception (I forgot) has also another advantage: it introduces "namespaced" error messages, for example you could do:

throw new Elk_Exception('Calendar.event_title_missing', false);

or:

throw new Elk_Exception(array('Calendar', event_title_missing'), false);

in both cases, what the class does is:

  1. loadLanguage('Calendar');
  2. use event_title_missing for the index (the same as usual).

you can see the _cleanMessage method

emanuele45 avatar Oct 05 '16 13:10 emanuele45

New hash after rebase: 2dc9391026de9cd85c3b70f9b81d704c47c28081 (code sent with PR #2715

emanuele45 avatar Oct 07 '16 11:10 emanuele45