craftcms/element-api

Exception handling improvements

benjamin-smith opened this issue · 4 comments

benjamin-smith commented

Right now, if an exception is thrown while processing an Element API HTTP request, the following code block is triggered:

element-api/src/controllers/DefaultController.php

Lines 165 to 174 in cb522b8

} catch (\Throwable $e) {
$data = [
'error' => [
'code' => $e instanceof HttpException ? $e->statusCode : $e->getCode(),
'message' => $e->getMessage(),
]
];
$statusCode = $e instanceof HttpException ? $e->statusCode : 500;
$statusText = $e->getMessage();
}

  • The error message is output to the client, regardless of the devMode setting. I've got info leaking from my API that I would like to keep private for security reasons.
  • The response is cached, regardless if an error occurred or not (I can see situations where not caching the error would also be bad, and could drive up server load, but overall my personal expectation would be this is not cached so temporary errors such as failed external network requests don't break an API for an extended period of time):

    element-api/src/controllers/DefaultController.php

    Lines 197 to 206 in cb522b8

    // Cache it?
    if ($cache) {
    if ($cache !== true) {
    $expire = ConfigHelper::durationInSeconds($cache);
    } else {
    $expire = null;
    }
    /** @noinspection PhpUndefinedVariableInspection */
    $cacheService->set($cacheKey, $response->content, $expire);
    }
akselikap commented

I ran into this issue in a different context. I am in the process of migrating a Craft 2 site to Craft 3. Craft 3 deprecates the "locale"-parameter we used for making our element api endpoints support multiple locales, like this:
$locale = craft()->request->getParam('locale', 'en'); return [ 'elementType' => 'Entry', 'criteria' => [ 'section' => 'someSection', 'locale' => $locale ],

Now we use "site" instead to do that but this issue affects that. Earlier we were able to call our API endpoints with non-existing locales and just receive an empty data array. However now that we are using site, we get a 404 error:
{"error":{"code":404,"message":"Invalid site handle: fr"}}.
I am fairly certain that this is directly linked to this same issue.

brandonkelly commented

Element API 2.7.0 is out now, which only caches 200 responses.

benjamin-smith commented

@brandonkelly I can open a separate issue with the details of the error reporting leak, unless that was also addressed in 2.7.0?

brandonkelly commented

@benjamin-smith Just released Element API 2.8.0, which no longer includes the exception message unless it’s an instance of yii\base\UserException (which indicates that the message is user-friendly).

  • 🎉1