Item12987: Foswiki returns "detected an internal error" with code 200, not 500

I noticed that the FoswikiOrgPlugin rest handler was failing with "Foswiki detected an internal error" but doesn't set a status code, so it's reported as 200 - successful transaction.

Content-Length: 194
Content-Type: text/plain; charset=iso-8859-1
Date: Mon, 11 Aug 2014 02:30:18 GMT
Server: Apache/2.2.26 (FreeBSD) SVN/1.8.8 mod_wsgi/3.4 Python/2.7.6 PHP/5.4.26 mod_ssl/2.2.26 OpenSSL/0.9.8y DAV/2 mod_fcgid/2.3.9

Body

Foswiki detected an internal error - please check your Foswiki logs and webserver logs for more information.

Can't locate Foswikipath in @INC (@INC contains: path path path path path path path)

The underlying cause of the crash is because the transaction hit during the pseudo-install run, and foswiki was incompletely installed. However the status should be a 500, not a 200.

-- GeorgeClark - 11 Aug 2014

I also noticed another possible error. Engine.pm, if it catches an error, it returns 500 to the caller, but without an error, it returns the request object. The caller (prepare()) treats whatever is returned as a request object. Engine should probably return undef.

-- GeorgeClark - 11 Aug 2014

Can you reproduce that error? The stack trace is pretty much essential to identify where the error is raised.

If Foswiki survives to the point where the REST handler function is called, then there is a try..catch wrapping the call that handles an Error::Simple. It does not, however, handled a simple Error as would be raised in the event of a die (an untrapped die might happen in 3rd party code), and that probably ought to be a catch Error rather than a catch Error::Simple. I changed most catches over; I would only have left that one because unit tests or code depended on it in some way.

-- CrawfordCurrie - 05 Nov 2014

The 200 vs. 500 is fixed. So no error there. I left the task open though because by code inspection, I believe that Engine.pm has an error. Trace what happens in Foswiki::Engine::prepare(). On a "catch" clause, it returns

return $e->{status};  or it might
return 500; 

But with success it returns

return $req;

The caller of prepare assumes any return is a request object. It doesn't test to see if it got a valid object, it just uses it. That just must be wrong somehow.

    my $req  = $this->prepare();
    if ( defined $req ) {
        my $res = Foswiki::UI::handleRequest($req);

and then Foswiki::UI::handleRequest uses $req without validating it:

   my $req = shift;
...
   my $dispatcher = $Foswiki::cfg{SwitchBoard}{ $req->action() };

-- GeorgeClark - 05 Nov 2014

I think that if (defined $req) { in run() should be if (ref($req)) {

-- CrawfordCurrie - 07 Nov 2014

Commenting is disabled while not logged in