PhpGt / WebEngine

Minimalistic, ergonomic PHP toolkit.
https://www.php.gt/webengine
MIT License
26 stars 6 forks source link

Page not found causes DalEl error #96

Closed 8w closed 11 years ago

8w commented 11 years ago

I've just noticed that if I hit an invalid URL in our app, the _Common.php still gets run (which might be fine?) but that there is then an error/ exception in the DalEl class - which means the 404 page doesn't get presented, the user gets a blank (white) screen.

Stack trace is:

[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP   9. ApiEl->getByUserID() _Common.php:107
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  10. ApiEl->__call() _Common.php:107
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  11. call_user_func_array() PHP.Gt/Framework/Component/ApiEl.php:78
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  12. Api->apiCall() PHP.Gt/Framework/Component/ApiEl.php:78
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  13. call_user_func_array() PHP.Gt/Framework/Component/Api.php:87
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  14. DalEl->__call() PHP.Gt/Framework/Component/Api.php:0
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  15. DalEl->query() PHP.Gt/Framework/Component/DalEl.php:47
[Tue Jun 11 19:34:19 2013] [warn] [client 127.0.0.1] mod_fcgid: stderr: PHP  16. PDOStatement->bindParam() PHP.Gt/Framework/Component/DalEl.php:106
g105b commented 11 years ago

Thanks for the issue, I had noticed this some time ago but forgotten all about it!

The error was being caused by trying to access the go parameters on a 404 page, as they are not all sent to the PageCode when the page doesn't exist.

The PHP.Gt errors are fixed, by sending EmptyObject instead, where appropriate, but your app will still need a tweak to fix it, as you'll still probably be assuming that $dom["something"] will be returning a DomEl.

To get around this, you'll simply have to extract the non-critical part of the _Common PageCode to the endGo function, which is called after the response is fully built (and so on 404 errors, the code will never fire). I had a look at your _Common.php, and it should look something like the following:

public function go($api, $dom, $template, $tool) {
    // Check if user has clicked logout button.
    if(isset($_REQUEST["Logout"])) {
        error_log("User-initiated logout of appUserID " . $_SESSION["appUserID"]);
        $this->logout();
        header("Location: /");
        exit;
    }

    $this->_security = $tool["Security"];
    $this->_pageReq = strtok($_SERVER['REQUEST_URI'], '?');
    $this->_pageReq = strtok($this->_pageReq, '#');
    $this->_pageReq = str_replace("/", "\/", $this->_pageReq);

    $appUser = $this->checkUser(
        $tool["User"], 
        $api["AppUser"], 
        $dom, 
        $template["Tickler"]
    );
}

// All of the code that *doesn't* need to be fired on 404 pages:
public function endGo($api, $dom, $template, $tool) {

    $dom["body"]->appendToAttribute("data-version", $appUser["ID"]);

    // Allows a mentor to view the plan of a user.
    if(isset($_GET["MentorViewExit"])) {
        error_log("Mentor " . $_SESSION["appUserID"] 
            . " stopping viewing plan of " . $_SESSION["planOwnerID"]);

        $_SESSION["planOwnerID"] = $_SESSION["appUserID"];

    }
        ...
        ...
        ...
}
8w commented 11 years ago

Hmm sounds a bit clunky! So all _commons will have to follow this format?

Isn't there a more elegant way to handle it? Surely if it's a 404 we shouldnt be running _common anyway? On 12 Jun 2013 09:39, "Greg Bowler" notifications@github.com wrote:

Thanks for the issue, I had noticed this some time ago but forgotten all about it!

The error was being caused by trying to access the go parameters on a 404 page, as they are not all sent to the PageCode when the page doesn't exist.

The PHP.Gt errors are fixed, by sending EmptyObject instead, where appropriate, but your app will still need a tweak to fix it, as you'll still probably be assuming that $dom["something"] will be returning a DomEl.

To get around this, you'll simply have to extract the non-critical part of the _Common PageCode to the endGo function, which is called after the response is fully built (and so on 404 errors, the code will never fire). I had a look at your _Common.php, and it should look something like the following:

public function go($api, $dom, $template, $tool) { // Check if user has clicked logout button. if(isset($_REQUEST["Logout"])) { error_log("User-initiated logout of appUserID " . $_SESSION["appUserID"]); $this->logout(); header("Location: /"); exit; }

$this->_security = $tool["Security"];
$this->_pageReq = strtok($_SERVER['REQUEST_URI'], '?');
$this->_pageReq = strtok($this->_pageReq, '#');
$this->_pageReq = str_replace("/", "\/", $this->_pageReq);

$appUser = $this->checkUser(
    $tool["User"],
    $api["AppUser"],
    $dom,
    $template["Tickler"]
);}

// All of the code that doesn't need to be fired on 404 pages:public function endGo($api, $dom, $template, $tool) {

$dom["body"]->appendToAttribute("data-version", $appUser["ID"]);

// Allows a mentor to view the plan of a user.
if(isset($_GET["MentorViewExit"])) {
    error_log("Mentor " . $_SESSION["appUserID"]
        . " stopping viewing plan of " . $_SESSION["planOwnerID"]);

    $_SESSION["planOwnerID"] = $_SESSION["appUserID"];

}
    ...
    ...
    ...}

— Reply to this email directly or view it on GitHubhttps://github.com/g105b/PHP.Gt/issues/96#issuecomment-19312918 .

g105b commented 11 years ago

The way I see it is if you have code in the _Common, you definitely want it to fire on every request no matter what, but if part of the code relies on part of the page being there, you'll either have to make your own checks in the code, or put the code in endGo so it doesn't fire if an HttpError occurs.

Can't think of a better solution at the moment, have you got any ideas? -----Original Message----- From: admin-finanscapes notifications@github.com Date: Wed, 12 Jun 2013 10:53:18 To: g105b/PHP.GtPHP.Gt@noreply.github.com Reply-To: "g105b/PHP.Gt" reply@reply.github.com Cc: Greg Bowlergreg.bowler@g105b.com Subject: Re: [PHP.Gt] Page not found causes DalEl error (#96)

Hmm sounds a bit clunky! So all _commons will have to follow this format?

Isn't there a more elegant way to handle it? Surely if it's a 404 we shouldnt be running _common anyway? On 12 Jun 2013 09:39, "Greg Bowler" notifications@github.com wrote:

Thanks for the issue, I had noticed this some time ago but forgotten all about it!

The error was being caused by trying to access the go parameters on a 404 page, as they are not all sent to the PageCode when the page doesn't exist.

The PHP.Gt errors are fixed, by sending EmptyObject instead, where appropriate, but your app will still need a tweak to fix it, as you'll still probably be assuming that $dom["something"] will be returning a DomEl.

To get around this, you'll simply have to extract the non-critical part of the _Common PageCode to the endGo function, which is called after the response is fully built (and so on 404 errors, the code will never fire). I had a look at your _Common.php, and it should look something like the following:

public function go($api, $dom, $template, $tool) { // Check if user has clicked logout button. if(isset($_REQUEST["Logout"])) { error_log("User-initiated logout of appUserID " . $_SESSION["appUserID"]); $this->logout(); header("Location: /"); exit; }

$this->_security = $tool["Security"];
$this->_pageReq = strtok($_SERVER['REQUEST_URI'], '?');
$this->_pageReq = strtok($this->_pageReq, '#');
$this->_pageReq = str_replace("/", "\/", $this->_pageReq);

$appUser = $this->checkUser(
    $tool["User"],
    $api["AppUser"],
    $dom,
    $template["Tickler"]
);}

// All of the code that doesn't need to be fired on 404 pages:public function endGo($api, $dom, $template, $tool) {

$dom["body"]->appendToAttribute("data-version", $appUser["ID"]);

// Allows a mentor to view the plan of a user.
if(isset($_GET["MentorViewExit"])) {
    error_log("Mentor " . $_SESSION["appUserID"]
        . " stopping viewing plan of " . $_SESSION["planOwnerID"]);

    $_SESSION["planOwnerID"] = $_SESSION["appUserID"];

}
    ...
    ...
    ...}

— Reply to this email directly or view it on GitHubhttps://github.com/g105b/PHP.Gt/issues/96#issuecomment-19312918 .


Reply to this email directly or view it on GitHub: https://github.com/g105b/PHP.Gt/issues/96#issuecomment-19342909

8w commented 11 years ago

I see the problem - and haven't come up with an elegant solution!

Are there other error scenarios that would happen the same way, or is 404 a special case? From an application perspective it's special, on the basis there is no page or functionality to be processed - hence the suggestion that _common shouldn't fire on 404s. But I don't know whether that's ignoring a swathe of other scenarios that are handled within php.gt? I guess things like server running out of disk space or unable to connect to the db? (though a db issue would be thrown as an exception in the application call stack I guess, rather than being handled directly by php.gt ).

If there are a series of error situations like 404 then maybe it would be more explicit/ clearer to have a goError() method that's called on _common instead of go()? It would be easier to explain in the docs and the app code would be more readable/ obvious. On 15 Jun 2013 09:19, "Greg Bowler" notifications@github.com wrote:

The way I see it is if you have code in the _Common, you definitely want it to fire on every request no matter what, but if part of the code relies on part of the page being there, you'll either have to make your own checks in the code, or put the code in endGo so it doesn't fire if an HttpError occurs.

Can't think of a better solution at the moment, have you got any ideas? -----Original Message----- From: admin-finanscapes notifications@github.com Date: Wed, 12 Jun 2013 10:53:18 To: g105b/PHP.GtPHP.Gt@noreply.github.com Reply-To: "g105b/PHP.Gt" reply@reply.github.com Cc: Greg Bowlergreg.bowler@g105b.com Subject: Re: [PHP.Gt] Page not found causes DalEl error (#96)

Hmm sounds a bit clunky! So all _commons will have to follow this format?

Isn't there a more elegant way to handle it? Surely if it's a 404 we shouldnt be running _common anyway? On 12 Jun 2013 09:39, "Greg Bowler" notifications@github.com wrote:

Thanks for the issue, I had noticed this some time ago but forgotten all about it!

The error was being caused by trying to access the go parameters on a 404 page, as they are not all sent to the PageCode when the page doesn't exist.

The PHP.Gt errors are fixed, by sending EmptyObject instead, where appropriate, but your app will still need a tweak to fix it, as you'll still probably be assuming that $dom["something"] will be returning a DomEl.

To get around this, you'll simply have to extract the non-critical part of the _Common PageCode to the endGo function, which is called after the response is fully built (and so on 404 errors, the code will never fire). I had a look at your _Common.php, and it should look something like the following:

public function go($api, $dom, $template, $tool) { // Check if user has clicked logout button. if(isset($_REQUEST["Logout"])) { error_log("User-initiated logout of appUserID " . $_SESSION["appUserID"]); $this->logout(); header("Location: /"); exit; }

$this->_security = $tool["Security"]; $this->_pageReq = strtok($_SERVER['REQUEST_URI'], '?'); $this->_pageReq = strtok($this->_pageReq, '#'); $this->_pageReq = str_replace("/", "\/", $this->_pageReq);

$appUser = $this->checkUser( $tool["User"], $api["AppUser"], $dom, $template["Tickler"] );} // All of the code that doesn't need to be fired on 404 pages:public function endGo($api, $dom, $template, $tool) {

$dom["body"]->appendToAttribute("data-version", $appUser["ID"]);

// Allows a mentor to view the plan of a user. if(isset($_GET["MentorViewExit"])) { error_log("Mentor " . $_SESSION["appUserID"] . " stopping viewing plan of " . $_SESSION["planOwnerID"]);

$_SESSION["planOwnerID"] = $_SESSION["appUserID"];

} ... ... ...}

— Reply to this email directly or view it on GitHub< https://github.com/g105b/PHP.Gt/issues/96#issuecomment-19312918> .


Reply to this email directly or view it on GitHub: https://github.com/g105b/PHP.Gt/issues/96#issuecomment-19342909

— Reply to this email directly or view it on GitHubhttps://github.com/g105b/PHP.Gt/issues/96#issuecomment-19493103 .