Validation errors on upload content should return a 4xx

[chrisvanrun]

Was working on testing an upload script and got back the following:

IMHO a client-side error is more appropriate here: for instance 415 Unsupported Media Type

[jmsmkn]

Err, 500 error means that you broke it and there will be an unhandled exception, probably this (https://grand-challenge.sentry.io/issues/4725383151/?project=303639&query=%21logger%3Acsp+is%3Aunresolved&referrer=issue-stream&statsPeriod=30d&stream_index=1)? There are other similar ones there. Can you provide a test in GC to reproduce?

[chrisvanrun]

I found out wat went wrong via Sentry initially.

Am I reading it wrong? It seems like a ValidationError on the content of the patch (i.e. a JSON schema not matching) results in a 500 error?

[jmsmkn]

The problem is likely that the validation error is raised in the incorrect place, is then unhandled. A test would reveal it.

[chrisvanrun]

Possibly sidely related to:

• Validation errors on job creation are not handled #3139

[amickan]

Yes, common issue. For display sets, this is solved by doing validation for file type interfaces in an async task in an atomic block. If validation fails, we send a notification to the user with the error message and the CIV does not get created. For archives items and algorithm jobs, we do the validation synchronously, but don't catch the validation error appropriately.

One way of validating files would be good. As such I think this ties in with https://github.com/DIAGNijmegen/rse-roadmap/issues/274

I'm now wondering if doing this asynchronously is really necessary. It would arguably be more user friendly if we validate the file contents against any possible json schema synchronously and return the error to the user in the form. That should be fast enough to do synchronously. For non json files we don't validate anything (except for checking wether the upload is complete), so no need for an asynch task for this either. Am I missing something?

[jmsmkn]

It needs to be async for multi-X JSON files as you can have things with millions of entries, you could DOS the API layer with that if it were in place. Same for things in values as we allow large inputs at the HTTP layer.

[amickan]

Ok, yes, asynch it is then. The algorithm and archive item code base need to be updated then. Best to have one place where this is all handled and then use that everywhere. Not sure how far I'll get before my vacation, but as I said this is anyway part of the larger pitch I linked above. Appetite feels big for this pitch with all the issues we've been encountering lately.