Handle input errors in the Web API

[cbenz]

EDIT by @fpagnoux:

As a user of the OpenFisca web API who sends a wrong input, I get a clear message that explicits why my input is not valid, So that I can fix it easily

---

Follows openfisca/openfisca-france#716

Problem is explained in the referenced issue.

We could catch ValueError around this line, but ValueError is not enough specific and this could lead to abusive exception catches.

So I suggest introducing a specific exception named PeriodError or PeriodMismatch, and catch it.

CC @fpagnoux

• apply this fix to other scenario.new_simulation calls
• test that error

[fpagnoux]

We could catch ValueError around this line, but ValueError is not enough specific and this could lead to abusive exception catches.

What kind of ValueError could be abusively catched ? What is the worst thing that can happen ?

I guess the user could get an answer that gives him internal details about an error which is not related to what they sent as an input. This is not a big deal for me. And the benefit of catching all is clear in many cases.

I'm not against introducing more specific errors, but the cost is higher: PeriodMismatch is not the only case when we would like to transmit the error to the API. We have to deal with all the cases, publish a Breaking change on core, etc.

I think @MattiSG implemented a catch all in the formula route. Thats seems to be the most pragmatic thing to do for me right now.

[cbenz]

I'm OK too to expose internal errors to the user.

So the try/except would be placed not around scenario.new_simulation but at a more higher lever, like in /formula endpoint.

Also we must find a way to be notified. Currently emails are sent only for uncaught exceptions. This can be changed, of course.

[fpagnoux]

Also we must find a way to be notified. Currently emails are sent only for uncaught exceptions. This can be changed, of course.

Not sure we want to get an email each time someone sends a wrong input :). Logging the errors to be able to monitor them could be nice though.

[cbenz]

Yes, we want to be notified when there is a real error, not an input error :) But with a catchall it's impossible.

🤔

I distinguish unexpected errors (triggered by programming mistakes) from managed errors (triggered by input errors).

• Either we accept a catchall knowing that we can't be notified anymore because we can't distinguish unexpected errors from managed errors,
• or we add specific named exceptions like PeriodError and consider them as managed errors, every other ones being unexpected, uncaught and trigger an email

Since logs are less intrusive, we can log more things than in emails.

[fpagnoux]

Do you have examples of errors we have been able to catch with this system in the past ?

Were there errors that where actually not concerning the client, but only internal business ?

[cbenz]

No, I don't have any examples, I was exposing my thinking about exception handling and the consequences of the 2 solutions.

Really, I'm OK to go for the catchall and rely on community feedback if false positives occur. Especially given it's not a global catchall: only scenario.new_simulation is between try/except in #111

[MattiSG]

we want to be notified when there is a real error, not an input error :)

I don't think we want to be notified on every API failure, honestly. We should definitely have logs and have a look at them on a regular basis, but I don't think we should get paged on API failure as long as we have no SLA. Unless we provision for proper availability in order to handle those notifications in a timely fashion, this will just end up as a pile of spam.