rcpch / digital-growth-charts-server

RCPCH's open source Digital Growth Chart API
https://growth.rcpch.ac.uk/
GNU Affero General Public License v3.0
16 stars 11 forks source link

500 error if height >250 cm #200

Closed eatyourpeas closed 5 months ago

eatyourpeas commented 8 months ago

There is no validation error for heights >250cm and this leads to a 500 error.

raspberrycodes commented 8 months ago

Thank you for raising this, I will keep an eye on it and will any raise issues to these repos in the future. :) Aurora

raspberrycodes commented 6 months ago

Following a discussion I had with @pacharanero on 29-Feb-2024, we have ironed out some further information about what has been observed with this bug that may be useful in finding a fix.

Firstly, the issue we were experiencing was 'Internal Server Error for maternal/paternal height >240.27'. Here is a screenshot of what I was getting in postman when making this call: image

As you can see, we get a 500 back which isn't nice for us to process and doesn't align with the expected behaviour set out by the Height, Weight, and OFC calls.

I couldn't find any documentation on validation constants for the maternal and paternal height fields, so I thought I found that it was a limit between 50.00cm and 240.27cm by messing around with the API. However, as you will see further down, I am beginning to think that these are not specified and I just got lucky finding those?

The plot thickens... my colleague Ryan found that it wasn't just because we were passing in a parental height value that exceeded the limit, but because in certain circumstances if the maternal and paternal heights passed to the midparental height API endpoint were such that a centile >100 would be calculated, this is when we would get the 500 returned.

For example, here is an example where we passed in a paternal height greater than 240.27, but the API responded successfully: image

And here we pass in values that cause it to break and generate a centile >100: image

Discussion with @pacharanero @pacharanero informed us that the team have been working on the server and python package to upgrade the python version and that he has been looking at the validation limits surrounding parental heights to see where this knock-on effect is generating centiles >100.

eatyourpeas commented 6 months ago

This may already be fixed in #201 There are actually 2 issues here.

  1. For complicated reasons there was a bug in the centiles which came to light when bumping pydantic from v1 to v2. In essence, the project board had asked us to return only integers for centiles, but of course this is not possible if the centile is >99.6 or <0.4. It is not actually possible to have a centile over 100, but in situations where centiles were >99 or <1 a float was returned, not an integer and this is what broke pydantic. An inconsistency in the code on my part. Pydantic V2 is much more pedantic than V1 and it came to light following a version bump that led to failing tests (thank goodness for pytest). I have therefore refactored the package to return floats (to 1 decimal place) for all centiles which should fix this, as well as updated all the unit tests to reflect this. I have also changed some of the upper limits in heights which are all documented in the PR.
  2. Thresholds are complex because of course they are age-dependent. A 23 wk gestation infant may have a weight only of a few hundred grams, where as a teenager in the obesity service might have a weight of 200kg. It is hard to validate this on absolute values therefore, particularly if you are entering data into a form and the age of the person is not yet know until you submit. The project board asked us not to implement upper and lower limits of 'normal', rather to report back to the users calculated values even for these abnormal values but attach a warning - this is what you see in the error strings. We chose 2cm for length because it seemed likely that anything down there would be a user who had entered a height/length in metres rather than centimeters. Another way to do this is to pick an SDS that is obviously abnormal and use that as a validation error - this has the benefit of being age and sex and measurement method specific. As I say though, some children with skeletal dysplasias are very small, and some people with obesity awaiting bariatric surgery are very large. In my bariatric clinic +6 SDS is not actually hugely unusual. Where would you put your cut off? In the constants folder in the python package (RCPCHGrowth) where all the calculations actually happen (separate repository and available on pypi) we have taken our thresholds from the guiness book of records. I don't know the exact answer but what we have to my mind is workable. Note the 500 errors you were getting were indeed a bug and a PR is sitting there to close: forgive me for this, just other things in that that need reviewing beforehand such as python version bump etc.

I hope that helps.