rcpch / digital-growth-charts-react-component-library

A typescript React library for displaying RCPCH Digital Growth Charts from API data
https://growth.rcpch.ac.uk
MIT License
7 stars 9 forks source link

Error Handling - Measurement list only containing errors #78

Closed dmc-cambric closed 4 months ago

dmc-cambric commented 6 months ago

Hi Team,

Ran into an issue when passing measurements containing errors to the chart component, wondering if you could advise.

Background:

I've removed hardcoded filtering out of measurements with API errors. Currently all measurements that have received a response from the API are sent directly to the charting component whether or not the API response object contains errors.

Previously I was filtering out measurements containing these API errors, determining error state by checking the following fields for values:

json.measurement_calculated_values.corrected_measurement_error, json.measurement_calculated_values.chronological_measurement_error, json.plottable_data.centile_data.chronological_decimal_age_data.observation_value_error, json.plottable_data.centile_data.corrected_decimal_age_data.observation_value_error, json.plottable_data.sds_data.chronological_decimal_age_data.observation_value_error, json.plottable_data.sds_data.corrected_decimal_age_data.observation_value_error

The Issue:

After removing the previous filtering of measurements with errors:

  1. When I pass a list of measurements to the chart component, if at least one of them contains an error, it results in the chart component putting errors in the console log.

  2. When I pass a list of measurements to the chart component, if all of them contain errors, it results in the charting component breaking + putting errors in the console log. (see attached screenshot)

I've also included a measurement containing an error, one that would previously be getting filtered out of the measurements list.

Request:

Looking to clarify if this is working as intended, or if the chart component should be processing these errors internally.

In situations where all the patient's measurements have errors, we show a placeholder separate to the charting component. I'm wondering if this should be implemented internally within the chart component (i.e. the chart component shows its own placeholder message describing why it can't render the chart), or if the chart component could have some sort of 'error callback' that we can hook into for displaying a placeholder with an appropriate description.

Any advice you can provide is much appreciated!

Using Ver 6.1.15

Cheers, Dan

image

api-data.json

eatyourpeas commented 6 months ago

Thanks @dmc-cambric I noticed that errors what not pulling through to the tool tips recently so this is something I am grateful here to you noticing.

I have started on som fixes which will go into a new version 7 where we are looking at a refresh on some aspects of the charts. This will be a priority.

eatyourpeas commented 5 months ago

Just starting to look at this - thanks for raising

The <RCPCHChart/> component is wrapped in an <ErrorBoundary/> component (thank you to @chvanlennep for the innovation). This retrieves errors that would prevent that chart from rendering and hopefully mostly are for situations when the server is unavailable or the data are unplottable. Most of that error handling though should have been addressed upstream.

Any errors to do with age calculation or measurements which would be plottable but maybe need a health warning will not be caught here, but should be signposted in the tooltips in the charts.

The message you have posted as an example is changing - in the next update we will be plotting preterm babies >= 22 weeks, even when reference data does not exist. But other errors, such as requesting weight in Turner and so on should be caught.

image image

Have you found errors that you would prefer were handled or implemented differently? Do you have any suggestions on how we might better handle errors?

dmc-cambric commented 4 months ago

These changes brought in v7.0.0 seem really handy. I don't have any immediate ideas around changes to error handling, although if the we (or clients) come up with any, will be happy to raise separately