Summary

Caught DoesNotExist exceptions in WebAPI, now raises 404

Review Request #3041 — Created April 3, 2012 and discarded Jan. 22, 2013, 11:22 p.m.

Information

Owner

ammok

Repository

Djblets

Branch

Bugs

2551

Depends On

Reviewers

Groups

djblets

People

Description

This addresses issue 2551.

Caught the DoesNotExist exceptions in the web API, and raised a HTTP 404 exception instead.

Unfortunately, it looks like I missed a space at the end of one line.
The (potential) problem I have with this one is that it will catch any DoesNotExist exception (rather than just the one associated with the review_request_resource model).

Testing Done

Testing done in Linux 3.2.13 with Chromium 18.0.

I visited local links that pointed to invalid review requests:
http://localhost:8080/api/review-requests/99/diffs/1/files/1/diff-comments/
[03/Apr/2012 22:37:30] "GET /api/review-requests/99/diffs/1/files/1/diff-comments/ HTTP/1.1" 404 139
http://localhost:8080/api/review-requests/999999999/screenshots/
[03/Apr/2012 22:40:35] "GET /api/review-requests/999999999/screenshots/ HTTP/1.1" 404 139

I'm not sure this is right. What's actually throwing the exception?

If we're that far into a list resource and we're getting an ObjectDoesNotExist, something went wrong. We've never needed to do this before, so I suspect the problem is best fixed elsewhere.

ammok April 4, 2012, 8:38 a.m.

ScreenshotResource uses WebAPIResource's get_list [line 671 of djblets/webapi/resources.py] which calls BaseScreenshotResource's get_queryset [line 2828 in rb], which calls review_request_resource.get_object [line 6223 in rb] (which is why I have to catch a more generic DoesNotExist error).

The exception is being thrown when queryset.get is called on lines 6234 of reviewboard/webapi/resources.py in get_object. Would raising an Http404 exception as in r/3040 be the recommended solution (it does differ from methods that use DOES_NOT_EXIST in that it's not directly part of the Web API)?

Status:: Discarded