opensafely-core / airlock

Other
0 stars 0 forks source link

Handle bad csvs #420

Closed rebkwok closed 2 weeks ago

rebkwok commented 2 weeks ago

Fixes #416

simple-datatables can only render tables with equal numbers of columns in each row. If a CSV output file has different columns per row (usually because it's not proper tabulated data), the datatable will fail to initialised. This was resulting in the page appearing to hang on the "loading ..." message.

This is addressed in a couple of ways: 1) Adds a setTimeout in the JS which waits for 5 seconds, and if the datatable element hasn't been found by then, it just unhides the table element anyway (which will show the plain html table if datatables failed to load). 2) Checks the column counts per row in the CSV renderer, and stops datatables being used at all if the counts aren't the same (which avoids having to wait 5 seconds when we know there's going to be a problem).

This does mean that we're assuming all tables will take at most 5s to load, and we're reading all CSV rows in in the renderer. Given that we've now got a 5000 row limit on L4 CSV files, this shouldn't cause any problems.

rebkwok commented 2 weeks ago

If we weren't relying on the job-server datatables JS, it might be easier to figure out (I briefly tried, but gave up because there are other more pressing things to do).I think most CSVs will be displayable as long as they don't have different numbers of columns, so the timeout is just a fallback, as that'll only be for other weirdness that datatables can't handle.