1311977 - ResultSetViewSet.list "KeyError: ..." with invalid fromchange values

Status: RESOLVED INVALID

(Tree Management :: Treeherder: API, defect, P2)

Description

[Ed Morley [:emorley]]

We should handle this case gracefully (with an HTTP 400) not HTTP 500.

/treeherder.webapp.api.resultset:ResultSetViewSet.list

File "/app/.heroku/python/lib/python2.7/site-packages/rest_framework/views.py", line 463, in dispatch
File "/app/treeherder/webapp/api/utils.py", line 104, in use_jobs_model
File "/app/treeherder/webapp/api/resultset.py", line 60, in list
 exceptions:KeyError: u'8948474958936717641.owasp.org' 

parameters	
fromchange 	8948474958936717641.owasp.org

https://rpm.newrelic.com/accounts/677903/applications/14179733/traced_errors/70d13c62-9482-11e6-9c7c-b82a72d22a14_4260_8309

Comment 1

[Ed Morley [:emorley]]

It seems that for this and the related bugs, we really need some kind of jsonschema-like validation for API inputs. I've had a quick glance but it's not clear if Django rest framework has such a thing. Having to roll our own handling for this on every API endpoint seems a bit tedious. Any ideas?

Comment 2

[William Lachance (:wlach)]

(In reply to Ed Morley [:emorley] from comment #1)
> It seems that for this and the related bugs, we really need some kind of
> jsonschema-like validation for API inputs. I've had a quick glance but it's
> not clear if Django rest framework has such a thing. Having to roll our own
> handling for this on every API endpoint seems a bit tedious. Any ideas?

I'm not aware of anything. In general I think we should avoid writing custom endpoints like this, there should be no reason to now that we're not using datasource for new models.

I'm currently rewriting this endpoint as part of bug 1311185, I'll try to take care of this case then.

Comment 3

[Ed Morley [:emorley]]

I agree we shouldn't be writing custom endpoints in a post-datasource world, however some of the other 500s have come from our ORM endpoints too.

Comment 4

[Cameron Dawson [:camd]]

I think that Serializers are the suggested way (deserializing the incoming data) in DRF.  Though I've never done it, so I'm not certain all that's involved.  I believe we only have 2 creation endpoints to cover: Jobs and Resultsets.  So perhaps that's not too much work.

Comment 5

[Ed Morley [:emorley]]

This issue is for GETs too, so all endpoints.

Comment 6

[William Lachance (:wlach)]

The code that was triggering this is no longer active.