...

View Full Version : Building a better Error-Handler?



doubledee
11-12-2012, 08:49 PM
I am wrapping up v2.0 of my website, and the last thing I'm playing with is seeing if I can improve my Error-Handling code.

For each of my scripts, the Error-Handling Code looks similar to the sample below...



// ********************************
// Attempt to Retrieve Articles. *
// ********************************
if (isset($_GET['section']) && $_GET['section']){
// Section found in URL.
// Continue processing here...

}else{
// Section Not found in URL.
$_SESSION['resultsCode'] = 'ARTICLE_INDEX_NO_QUERY_STRING_2422';

// Set Error Source.
$_SESSION['errorPage'] = $_SERVER['SCRIPT_NAME'];

// Redirect to Display Outcome.
header("Location: " . BASE_URL . "/account/results.php");

// End script.
exit();

}//End of ATTEMPT TO RETRIEVE ARTICLES



So, for each script, I am basically storing the "Error Code" and the "Offending Script Name" into a SESSION variable, and then re-directing to my "results.php" script which does the following things...

1.) Displays an Error Message to the User
2.) Logs the Error into a Database Table
3.) Emails the Admin if the Error could not be written to the Database
4.) Emails the Admin with a similar Error Message so he/she can stay informed of User/System Errors


For #1, I just use a gigantic Case statement like this...


// No Query String.
case 'ARTICLE_INDEX_NO_QUERY_STRING_2422':
echo '<h1>No Section Chosen</h1>';
echo '<p>Please choose a Section to be viewed. (2422)</p>';
echo '<a class="button" href="' . BASE_URL . '/">Return to Home Page</a>';
break;



For #2, I just use a simple INSERT.


For #3 and #4, I send an HTML-formatted e-mail to the Admin.


Any thoughts or suggestions?


Everything seems to be working, however one issue I discovered is the fact that my "Error Messages" to the User are really "Outcome Messages" because they sometimes say "Hey, whatever you just did succeeded!!"

Because my "results.php" script bundles everything together, I noticed that I will be getting an e-mail every time someone posts a Comment to one of my Articles. (Imagine if the Admin for this site got an e-mail everytime someone posted a message?!) :eek: :eek:

Once I discovered that, I decided that my "Error-Handling" probably needs to be improved!!

However, because my Error-handling code is so intertwined with my entire code-base which is probably 10,000-15,000 lines of code, I need to be careful to not break things?!

For now, I am just seeing if there is a way that I can make it so I only send an e-mail to the Admin if there really is an ERROR, and not just every time something happens (e.g. User Logs In, User Friends Someone, User Posts Message, etc.).

Anyways, I am open to suggestions...

Sincerely,


Debbie

P.S. Please keep suggestions to Procedural Coding, because OOP-solutions will have to wait for v3.0 ;)

Fou-Lu
11-13-2012, 12:44 AM
Too much text to read.
Why not make use of set_error_handler, and issue trigger_error instead? This issues its own stack, so you can easily track down the file, line, context, etc entirely from the error handler itself. Easy to convert and rethrow as an errorException when it comes time to convert to OO.

doubledee
11-13-2012, 01:58 AM
Too much text to read.

What is "too much text to read"?

My original post?! :confused:



Too much text to read.
Why not make use of set_error_handler, and issue trigger_error instead? This issues its own stack, so you can easily track down the file, line, context, etc entirely from the error handler itself. Easy to convert and rethrow as an errorException when it comes time to convert to OO.[/QUOTE]

Because most of the Error-Handling code I have - as shown in the OP - are "logical" errors and not run-time errors, so set_error_handler wouldn't help.


Debbie

Fou-Lu
11-13-2012, 02:53 AM
Error handling will work just fine, that is what trigger_error is used for. Give it a E_USER_* level error to determine what to do with it. So simply use a logical decision on if an error has occurred, and trigger a user level error according to the error level.

doubledee
11-13-2012, 02:55 AM
Error handling will work just fine, that is what trigger_error is used for. Give it a E_USER_* level error to determine what to do with it. So simply use a logical decision on if an error has occurred, and trigger a user level error according to the error level.

Sorry, but I am not following you... :(

Also, I'm not sure if you are following what I currently have, although I believe I posted some code above.

Would more of what I currently have help?


Debbie

firepages
11-13-2012, 03:21 AM
Fou-Lu's suggestion is by far really the best way to go though perhaps its too late in V2 to start rewriting, thats your call.

for your existing code I can't see why you would get an email if there were was no error ... unless...

<?php
$_SESSION['resultsCode'] = 'ARTICLE_INDEX_NO_QUERY_STRING_2422';
?>
when its an error it should be e.g.
<?php
$_SESSION['errorCode'] = 'ARTICLE_INDEX_NO_QUERY_STRING_2422';
?>


I sounds like you issue an email for every resultsCode when you should probably do a check at that satge of your logic for resultsCode AND errorCode and act accordingly

doubledee
11-13-2012, 03:58 AM
Fou-Lu's suggestion is by far really the best way to go though perhaps its too late in V2 to start rewriting, thats your call.

Well, first I need to understand what he is recommending. (The built-in error-handling he mentioned has always been Greek to me, which is probably why I created what I have now?!)



for your existing code I can't see why you would get an email if there were was no error ... unless...

<?php
$_SESSION['resultsCode'] = 'ARTICLE_INDEX_NO_QUERY_STRING_2422';
?>
when its an error it should be e.g.
<?php
$_SESSION['errorCode'] = 'ARTICLE_INDEX_NO_QUERY_STRING_2422';
?>


Precisely!!!

The *main reason* I want to re-write my Error-Handling, is because as it stands now, the Admin (i.e. ME) gets an e-mail every time there is an outcome!

Add a Friend? (I get an e-mail?!)

Post a Comment below an Article? (I get an e-mail?!)

I *always* want to log "Failures" and "Successes" in my database - if I take the time to write the code for it. However, e-mails should only go out to the Admin when there is an Error, and so some action is probably need.


---------
Below is some more code that will hopefully better illustrate my current code-base...


"add_comment.php"


// Verify Insert.
if (mysqli_stmt_affected_rows($stmt3)==1){
// Insert Succeeded.
$_SESSION['resultsCode'] = 'COMMENT_MEMBER_COMMENT_ADDED_2052';

// Set values for Success Message.
$_SESSION['firstName'] = $firstName;
$_SESSION['heading'] = $heading;

// Notify Other Subscribed Members
notifySubscribersOfNewComment($dbc, $articleID, $commentID, $sessMemberID);

}else{
// Insert Failed.
$_SESSION['resultsCode'] = 'COMMENT_MEMBER_COMMENT_FAILED_2053';

}// End of VERIFY INSERT.


// Set Error Source.
$_SESSION['errorPage'] = $_SERVER['SCRIPT_NAME'];

// Redirect to Display Outcome.
header("Location: " . BASE_URL . "/account/results.php");

// End script.
exit();

(**Notice, too, that I have other random variables that I like to pass to my Error/Success Messages?! Currently, I always pass $_SESSION['resultsCode'] and $_SESSION['errorPage'], but it not uncommon for me to pass something off-the-wall to make the message more meaningful and personal.)


"results.php"


// Comment Added.
case 'COMMENT_MEMBER_COMMENT_ADDED_2052':
echo '<h1>Comment Submitted</h1>';
echo '<p>Thanks, '
. str2htmlentities($firstName)
. ', for sharing your thoughts! (2052)</p>';
echo '<p>Once approved, your comment will appear below the article:<br />
<span id="heading">' . '"'
. str2htmlentities($heading)
. '"</span></p><br />';
echo '<ul>
<li>
<a class="button2" href="' . BASE_URL . $returnToPage . '">Return to Article</a>
</li>
<li>or</li>
<li>
<a class="button2" href="' . BASE_URL . '/">Go to Home Page</a>
</li>
</ul>';
break;


// Comment Not Added.
case 'COMMENT_MEMBER_COMMENT_FAILED_2053':
echo '<h1>Comment Failed</h1>';
echo '<p>Your comment could not be added due to a system error.</p>';
echo '<p>Please contact the System Administrator. (2053)</p>';
echo '<a class="button" href="' . BASE_URL . $returnToPage . '">Return to Article</a>';
break;

(Of course, there is quite a bit more code in "results.php", but the code above shows you the "Error/Success Message" I display on the User's screen...)




I sounds like you issue an email for every resultsCode when you should probably do a check at that stage of your logic for resultsCode AND errorCode and act accordingly

That is the gist of it, however I am trying to find the easiest way to tweak my existing code base for v2.0 (When I hopefully go to OOP in v3.0, I can tackle this entirely differently if it makes sense.)

Right now, I estimate my website is maybe 30 scripts, 15,000 lines of code, and "results.php" is about 2,100 lines of code, with 1,600 of those lines being *customized* Error/Success Messages that I don't want to touch except for what we are talking about related to Screen Messages and E-mail Content.

Hope that helps better explain what I am trying to tackle?!

Sincerely,


Debbie

doubledee
11-13-2012, 04:15 AM
firepages & Fou-Lu,

If you guys are gonna help me, I guess I should "cough up" some more code...

Here are the guts of my "results.php" script... (No laughing!!!) :o



<?php
// Initialize Session.
session_start();

// Access Constants.
require_once('some_file.php');

// Connect to the database.
require_once('some_file.php');


// ********************************
// Initialize SESSION Variables. *
// ********************************
$resultsTitle = (isset($_SESSION['resultsTitle']) ? $_SESSION['resultsTitle'] : 'Results');
$resultsCode = (isset($_SESSION['resultsCode']) ? $_SESSION['resultsCode'] : 'DEFAULT_CATCHALL_OUTCOME_9999');
$errorPage = (isset($_SESSION['errorPage']) ? $_SESSION['errorPage'] : '');

$sessMemberID = (isset($_SESSION['sessMemberID']) ? $_SESSION['sessMemberID'] : '');

$regEmail = (isset($_SESSION['registrationEmail']) ? $_SESSION['registrationEmail'] : '');
$newEmail = (isset($_SESSION['newEmail']) ? $_SESSION['newEmail'] : '');

$firstName = (isset($_SESSION['sessFirstName']) ? $_SESSION['sessFirstName'] : '');
$heading = (isset($_SESSION['heading']) ? $_SESSION['heading'] : '');

$returnToPage = (isset($_SESSION['returnToPage']) ? $_SESSION['returnToPage'] : '/index.php');
$articleReturnPath = (isset($_SESSION['articleReturnPath']) ? $_SESSION['articleReturnPath'] : '/index.php');

$username = (isset($_SESSION['username']) ? $_SESSION['username'] : '');


// ************************
// Initialize Variables. *
// ************************
// LOOK
$emailTo = 'DebbiesEmail@mail.com';

$headers = "From: Admin <admin@MySite.com>\r\n";
$headers .= "MIME-Version: 1.0\r\n";
$headers .= "Content-Type: text/html; charset=ISO-8859-1\r\n";

$currTime = date('Y-m-d g:i:sa', time());

// Capture User's IP Address.
$ip = $_SERVER['REMOTE_ADDR'];

// Capture User's Hostname.
$hostName = gethostbyaddr($_SERVER['REMOTE_ADDR']);


// ********************
// Log Error Code. *
// ********************

// Build Query.
$q1 = "INSERT INTO z_result(code, page, member_id, ip, host_name, created_on)
VALUES (?, ?, ?, ?, ?, NOW())";

// Prepare statement.
$stmt1 = mysqli_prepare($dbc, $q1);

// Bind variables to query.
mysqli_stmt_bind_param($stmt1, 'ssiss', $resultsCode, $errorPage, $sessMemberID, $ip, $hostName);

// Execute query.
mysqli_stmt_execute($stmt1);

// Verify Insert.
if (mysqli_stmt_affected_rows($stmt1)!==1){
// Insert Failed.

// ************************************
// Email Admin about Failed Insert. *
// ************************************
$subject_FailedInsert = 'Re: Unloggable Website Error (' . $currTime . ')';

$body_FailedInsert = $currTime ."\n\n";
$body_FailedInsert .= "Dear Admin,\n\n";
$body_FailedInsert .= "A website error has occurred, but could not be logged in the database.\n\n";
$body_FailedInsert .= "Please take immediate action!!\n\n";

//LOOK
mail($emailTo, $subject_FailedInsert, $body_FailedInsert, $headers);

}// End of LOG ERROR CODE


// **************************
// Email Results to Admin. *
// **************************
$subject_Error = 'Re: Website #: ' . rand(5, 1000);

$body_Error = "<table>\n";
$body_Error .= " <tr><td colspan=\"2\">A website error has occurred...<br /><br /></td></tr>\n";
$body_Error .= " <tr><td>Date:</td><td>" . $currTime ."</td></tr>\n";
$body_Error .= " <tr><td>Results Code:</td><td>" . $resultsCode . "</td></tr>\n";
$body_Error .= " <tr><td>Error Page:</td><td>" . $errorPage . "</td></tr>\n";
$body_Error .= " <tr><td>Member ID:</td><td>" . $sessMemberID . "</td></tr>\n";
$body_Error .= " <tr><td>IP Address:</td><td>" . $ip . "</td></tr>\n";
$body_Error .= " <tr><td>Host Name:</td><td>" . $hostName . "</td></tr>\n";
$body_Error .= "</table>\n";


//LOOK
mail($emailTo, $subject_Error, $body_Error, $headers);

?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">

<head>
<!-- ################## DEBBIE ##################### -->
<!-- HTML Metadata -->
<title><?php echo $resultsTitle; ?></title>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />

<!-- Page Stylesheets -->
<link type="text/css" rel="stylesheet" href="/css/_main.css" />
<link type="text/css" rel="stylesheet" href="/css/_layout.css" />
<link type="text/css" rel="stylesheet" href="/css/top_menu.css" />
<link type="text/css" rel="stylesheet" href="/css/components.css" />
</head>

<body>
<div id="pageWrapper" class="clearfix">
<div id="pageInner">
<!-- BODY HEADER -->
<?php require_once(WEB_ROOT . 'components/body_header.inc.php'); ?>


<!-- MIDDLE COLUMN -->
<div id="pageMidCol_1">
<div id="boxMessage">
<?php

// ************************
// Display Results Code. *
// ************************
switch ($resultsCode){

//******************
//yy
// Not Logged In.
case 'COMMENT_USER_NOT_LOGGED_IN_2045':
echo '<h1>Not Logged In</h1>';
echo '<p>You must be logged in to add a comment. (2045)</p>';
echo '<a class="button" href="' . BASE_URL . '/account/log_in.php">Log In</a>';
break;

//yy
// Article Not Found.
case 'COMMENT_ARTICLE_NOT_FOUND_2046':
echo '<h1>Article Not Found</h1>';
echo '<p>The Article you chose was not found.</p>';
echo '<p>Please try again. (2046)</p>';
echo '<a class="button" href="' . BASE_URL . '/articles/index.php">Return to Article Listing</a>';
break;

// and so on, and so forth...



Hope that helps?!

Thanks,


Debbie

firepages
11-13-2012, 05:32 AM
honestly, I think you are going to have to do a search on your codebase .. (grep -r 'resultsCode' /path/to/www (or find in files in windows)) and anything that is actually an error change to `errorCode` , then in your results script check if `errorCode` or `resultsCode` has been passed and act accordingly, I assume you probably just need to check for that at the sending email stage ?

[The alternative would be to add some flag in the value of resultsCode indicating and error or a message but this is probably more work and still requires editing lots of files?]

I cant think of anything off of the top of my head that is ging to be an easier way out since the problem is in lots of places whichever way you go about fixing it?

your switch is a bit long winded and probably you would be better off searching an array of error codes ? not that it really matters either way.

Fou-Lu
11-13-2012, 05:41 AM
. . .no. . . more. . . code. Lol
Error handling is exceedingly easy to perform. Since you have specific criteria, that is what I would suggest passing to the trigger.


function errorHandler($errno, $errstr, $errfile, $errline)
{
$_SESSION['resultsCode'] = $errstr;
switch ($errno)
{
case E_USER_FATAL:
// email or whatever. I'd die here
die();
default:
header('Location: ' . ERROR_LOCATION);
die();
}
return true;
}

set_error_handler('errorHandler', E_USER_NOTICE | E_USER_WARNING | E_USER_FATAL);

if ($badDataIsTrue)
{
trigger_error('COMMENT_MEMBER_COMMENT_FAILED_2053', E_USER_WARNING);
}

Now you simply define ERROR_LOCATION on a script by script basis, and when triggered the error handler will assign the given value to the session, and redirect to the location under ERROR_LOCATION.
You are bound by only using the three error levels on a trigger. Technically you could take an E_USER_NOTICE level and assign results this way and modify the error handler to react accordingly. Personally I wouldn't do that, I'd just continue processing per normal for good runs, and present the error handler with failures only. I'd suggest that E_USER_FATAL is the only one you need to do something like issue an email, but the others may want to have a logging feature in place or whatever.

Custard7A
11-13-2012, 03:00 PM
The flaw in your current error system is a bit obvious, results.php doesn't even check that it's receiving an error. Heck, it doesn't seem to be checking very much at all, it just constructs everything and sends away. The only thing I see being checked is if the error has been logged in the database, which it is trying to log regardless of what has happened. This is probably the most inefficient script in existence. :(

Adding on top of that, you seem to be wanting to send an email on "errors" such as when somebody enters your index without a GET request for a "section". Why on earth would you want an email for that?? The redirect and notice is the most you can do in practical terms. You're basically spamming yourself; knowing afterwards that someone took away the GET is pointless, as is logging it.

If you're just concerned with a temporary fix, then you need to add checks around the error logging and email sending for something that is ONLY sent from resulting errors. In reality this needs a restructure with different logic entirely, such as what the others have suggested. You might want to also create your code a great deal more compartmentalized in v.3. If you create your code efficiently, then editing sections or making changes like these becomes a 5 minute hassle.

@ Fou-Lu : Because trigger_error uses actual PHP error levels, will it also populate the native error log? That could sure be useful.

Fou-Lu
11-13-2012, 05:06 PM
trigger_error will only work with E_USER level errors. But I believe these are also logged natively when triggered. Returning true on that handler though may block it from reaching the native handler, but I'd need to test it out to verify.
Typically if you want to force a log, I'd suggest using the error_log function.

doubledee
11-14-2012, 04:52 AM
I got home late tonight, so this will be short for now...



honestly, I think you are going to have to do a search on your codebase .. (grep -r 'resultsCode' /path/to/www (or find in files in windows)) and anything that is actually an error change to `errorCode` , then in your results script check if `errorCode` or `resultsCode` has been passed and act accordingly, I assume you probably just need to check for that at the sending email stage ?

[The alternative would be to add some flag in the value of resultsCode indicating and error or a message but this is probably more work and still requires editing lots of files?]

I cant think of anything off of the top of my head that is ging to be an easier way out since the problem is in lots of places whichever way you go about fixing it?

Today at work, this is what I came up with for a quick solution...

I could assume that all "results" are "Errors", and therefore need both Logging and an Email.

Then in my "results.php" code, I could make the above assumption the "default" behavior.

Then in any code where I am just displaying an "outcome" that is NOT an "error", I could add this...

I could add this...


$_SESSION['isEmailable'] = FALSE;



Somewhere in "results.php", I would assume that normally....


$_SESSION['isEmailable'] = TRUE;


...but in case where it is FALSE, then take a different action.

(This made sense in my head today at work. Not sure how articulate I'm being right before bedtime?!) :o



your switch is a bit long winded and probably you would be better off searching an array of error codes ? not that it really matters either way.

Each Error-Message is *customized*. (Although some day, I'm sure I can condense things down into "10 Flavors of Errors"?!)

That being said, how would an array solve anything?

Sincerely,


Debbie

doubledee
11-14-2012, 05:05 AM
Fou-Lou,



. . .no. . . more. . . code. Lol

Since I want/need customized Error Messages, I don't see a better way. (Plus if I had stuck all of that in my database, and my database puked, you'd get now Error Messages, right?!)




Error handling is exceedingly easy to perform.

Maybe for you, but your response below still has me confused... :(



Since you have specific criteria, that is what I would suggest passing to the trigger.



function errorHandler($errno, $errstr, $errfile, $errline)
{
$_SESSION['resultsCode'] = $errstr;
switch ($errno)
{
case E_USER_FATAL:
// email or whatever. I'd die here
die();
default:
header('Location: ' . ERROR_LOCATION);
die();
}
return true;
}

set_error_handler('errorHandler', E_USER_NOTICE | E_USER_WARNING | E_USER_FATAL);

if ($badDataIsTrue)
{
trigger_error('COMMENT_MEMBER_COMMENT_FAILED_2053', E_USER_WARNING);
}




In my code in previous posts, can you help me maps my code to your variables?

What is...

$errno ??

$errstr ??

$errfile ??

$errline ??


-----
How does this code work...


case E_USER_FATAL:
// email or whatever. I'd die here
die();
default:
header('Location: ' . ERROR_LOCATION);
die();



Like I said before, I think almost all of my "Error Handling" is just handling "logical" errors, or at least "predictable" ones.

I am not really handling for things like when "There is no database connection".

My code is handling things like "User hacked the URL and put in an invalid 'Section'." or "User already added Member as Friend." and so on...

I'm not following how the snippet above works or fires or relates to the samples I originally posted?! :confused:


In this code...


if ($badDataIsTrue)
{
trigger_error('COMMENT_MEMBER_COMMENT_FAILED_2053', E_USER_WARNING);
}


Where does $badDataIsTrue come from?!

And I have to add all of that code to every Error?

I don't mean to be a weeny, but if you could maybe show me how I would modify my code to incorporate what you are saying, that would really be helpful, because my tiny, pee-brain isn't really following your example... :(

(I am trying to write my own code, but obviously if I knew all about Error-Handling I wouldn't be asking fo help in the forum!!)




Now you simply define ERROR_LOCATION on a script by script basis, and when triggered the error handler will assign the given value to the session, and redirect to the location under ERROR_LOCATION.
You are bound by only using the three error levels on a trigger. Technically you could take an E_USER_NOTICE level and assign results this way and modify the error handler to react accordingly. Personally I wouldn't do that, I'd just continue processing per normal for good runs, and present the error handler with failures only. I'd suggest that E_USER_FATAL is the only one you need to do something like issue an email, but the others may want to have a logging feature in place or whatever.

Again, I'm getting lost...

Thanks for everyone trying to help so far!!


Debbie

firepages
11-14-2012, 05:09 AM
...
I could assume that all "results" are "Errors", and therefore need both Logging and an Email.


Well really thats effectively what already happens (though currently everything is a `result` rather than an `error`) and is the source of your current dilemma.



I could add this...
PHP Code:
$_SESSION['isEmailable'] = FALSE;


If you are going to go through your code and add stuff you would be better to set a flag saying 'its an error' or, do as, or similar to, my first suggestion , just cos its emailable does not mean its an error, and that again is the current issue you are having, if it walks and talks like a duck, flag it as a duck.
in results.php if you have an error flag you do 1 thing, if a result flag another.



That being said, how would an array solve anything?

It would not (other than be subjectively tidier), more a personal preference and just a comment.

Custard7A
11-14-2012, 01:39 PM
Firepages is right, logic like that is behind a lot of your problems. Consider your idea: You set up everything like it's an error, and only do the final outcomes depending on if it actually is. Hopefully, for the vast majority of times, your result will not be an error. You will not need an error set up in these cases, but you will still burdening your script with the assumption that it will be. Meaning this script could be running a lot smoother for pages not generating an error. You shouldn't need to assume anything in a good script.

It should be this simple..

In a main file:




if( !isset($_GET['section']) || strlen($_GET['section']) == 0) {

$_SESSION['Error'] = 2422; // Some error number, or something.

header("Location: ". BASE_URL ."/account/results.php"); }



In the results file:




// Check if an error has been set.
if( isset($_SESSION['Error'])) && $_SESSION['Error'] != 0) {

switch($_SESSION['Error']){ // Find what error it is.

case 2422: echo "Error: No section defined in ". $_ENV['REQUEST_URI']; break;

// Add new cases for any self-defined error you may use.

default: echo "No error was defined."; break;

} }



1 ). You wouldn't echo the message like that, that's just for simple example. If it was me I would call a function there that would then handle logging and emailing. Well, if it was me I wouldn't be doing anything like this, lol.

2 ). Your calculation on $_GET['section'] is completely ineffective, and wouldn't trigger your error handling if it was a empty string (but still set), or actually existed. Meaning your check is doing next to nothing to actually catch what you're trying to catch. I could change your url to /your_file.php?section=, or even /your_file.php?section=anything and your check would do nothing to stop that. Depending on how you're using $_GET['section'] after this check you may have a massive security hole there.

3 ). If I recall right, using header(Location: ) will return a redirect response code, and will preserve the original page location in the environment variable $_ENV['REQUEST_URI']. You should be able to use that instead of remembering $_SERVER['SCRIPT_NAME'] from the last page.

4 ). You are doing a lot of writing and asking a lot of questions. This isn't necessarily bad, but you're making it hard for anyone to help you. If you are serious about fixing your problems you should consider taking some of the previous advice to heart, and going to the PHP documentation or Google and doing a bit of reading. A little more effort to understand what is being suggested could go a long way, and save you more time in the end.

doubledee
11-17-2012, 07:38 PM
Firepages is right, logic like that is behind a lot of your problems. Consider your idea: You set up everything like it's an error, and only do the final outcomes depending on if it actually is. Hopefully, for the vast majority of times, your result will not be an error.

Not so.

I created a script called "results.php" to display a "result" or "outcome" message on the screen based on what a User did.

This includes "Error" messages like...


Comment Not Found
The Comment you chose was not found.
Please try again. (2328)

<Return to Article>


And "Success" messages like...


Comments Changed
Your article comments have been changed. (2330)

<Return to Article>



However, I will agree that the majority of my messages tend to deal more with when something goes wrong.




You will not need an error set up in these cases, but you will still burdening your script with the assumption that it will be. Meaning this script could be running a lot smoother for pages not generating an error. You shouldn't need to assume anything in a good script.

Maybe part of the problem here is "What constitutes an 'Error'?"

To me, an "Error" is anytime the User doesn't get their expected outcome, or when the script fails.

Typically my scripts deal with the following "errors"...

- No Value (i.e. NULL) was passed
- No record can be found for the submitted Value
- The Value has an Invalid Format
- User is trying to do something that is not allowed

All of my "Error-Handling" is not dealing with Compile or Fatal Errors, and in fact, I'm not sure I know how to do that?! :o

What I am calling "Error-Handling" involves going through every branch of my code and making sure I have some "Results/Outcome" message for *every* scenario possible, so that my code *always* ends gracefully with some Custom Message.

If my code is invalid, then it wouldn't run, so I guess I catch "Compile" errors during testing?

And maybe I am being naive, but I guess I assume that by going through every branch in my code, combined with doing obvious things like checking for Nulls and checking for proper Data-Types, that my code has a branch - and message too - to handle everything that can happen.

Does that make sense?

(I was hoping that the code I posted before would give all of you a good idea of how my entire scripts look...)




It should be this simple..

In a main file:




if( !isset($_GET['section']) || strlen($_GET['section']) == 0) {

$_SESSION['Error'] = 2422; // Some error number, or something.

header("Location: ". BASE_URL ."/account/results.php"); }



In the results file:


// Check if an error has been set.
if( isset($_SESSION['Error'])) && $_SESSION['Error'] != 0) {

switch($_SESSION['Error']){ // Find what error it is.

case 2422: echo "Error: No section defined in ". $_ENV['REQUEST_URI']; break;

// Add new cases for any self-defined error you may use.

default: echo "No error was defined."; break;

} }




But that is almost identical to what I have?! :confused:




1 ). You wouldn't echo the message like that, that's just for simple example. If it was me I would call a function there that would then handle logging and emailing. Well, if it was me I wouldn't be doing anything like this, lol.

Then what would you do?? :rolleyes:




4 ). You are doing a lot of writing and asking a lot of questions. This isn't necessarily bad, but you're making it hard for anyone to help you. If you are serious about fixing your problems you should consider taking some of the previous advice to heart, and going to the PHP documentation or Google and doing a bit of reading. A little more effort to understand what is being suggested could go a long way, and save you more time in the end.

And I said I didn't understand what was suggested above, so RE-reading the PHP Manual won't do a whole lot of good...


Debbie

Custard7A
11-18-2012, 01:31 AM
But that is almost identical to what I have?! :confused:


Kinda, but most importantly you're not checking if an error has occurred or not at the results.php.



Then what would you do?? :rolleyes:


Perhaps I would send bad GET queries a HTTP 404, or 400; Script errors might get a HTTP 500. I could make use of existing database framework, I would be using OOP.. etc. It's not that important what I would do, since that doesn't make what you're doing wrong.



And I said I didn't understand what was suggested above, so RE-reading the PHP Manual won't do a whole lot of good...


Yeah, the manual can be a bit harder to get your head around. Google can usually come up with some better entry-level tutorials though. I think my point is we're here to encourage and assist better web development, not to re-write people's scripts and send them on their way. There's enough of those requests from the Wordpress crowd, and it's actually counter-productive in the long run when they meet the same problem again.

doubledee
11-18-2012, 05:32 AM
Yeah, the manual can be a bit harder to get your head around. Google can usually come up with some better entry-level tutorials though. I think my point is we're here to encourage and assist better web development, not to re-write people's scripts and send them on their way. There's enough of those requests from the Wordpress crowd, and it's actually counter-productive in the long run when they meet the same problem again.

I never asked anyone to re-write my script.

I did ask for help understanding what was being suggested, as well as asking several questions for clarification that people haven't responded back to though...

There is a big difference between that and what you are implying...


Debbie

firepages
11-18-2012, 03:38 PM
But that is almost identical to what I have?!

yes because you simply switched results to error , you need to set either and check both
e.g.

if (mysqli_stmt_affected_rows($stmt3)==1){
// Insert Succeeded.
$_SESSION['resultsCode']['result'] = 'COMMENT_MEMBER_COMMENT_ADDED_2052';

// Set values for Success Message.
$_SESSION['firstName'] = $firstName;
$_SESSION['heading'] = $heading;

// Notify Other Subscribed Members
notifySubscribersOfNewComment($dbc, $articleID, $commentID, $sessMemberID);

}else{
// Insert Failed.
$_SESSION['resultsCode']['error'] = '2';
// Set Error Source.
$_SESSION['errorPage'] = $_SERVER['SCRIPT_NAME'];

// Redirect to Display Outcome.
header("Location: " . BASE_URL . "/account/results.php");
}// End of VERIFY INSERT.


& in results.php e.g.


$errors=array(
2=>array('msg'=>'you did this wrong','ID'=>'COMMENT_MEMBER_COMMENT_ADDED','gen_email'=>false),
4=>array('msg'=>'you did that wrong','gen_email'=>false),
8=>array('msg'=>'you did the other wrong','gen_email'=>true),
//etc
)

if($_SESSION['resultsCode']){
if($_SESSION['resultsCode']['result']){
//do whatever you want to do here
}
if($_SESSION['resultsCode']['error']){
//its an error you may have other things to do now?
//check $errors[$_SESSION['resultsCode']['error']]; for what
}
}


as for what is an error... whatever you class as an error but you posted here because you were getting emails for every member action, something I assume you want to avoid... the above is one (simplistic) way of modifying your existing code.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum