Closed Bug 1402308 Opened 7 years ago Closed 7 years ago

LandoAPI - Intercept the Exception and fail with a message if no right to receive a revision from Phabricator

Categories

(Conduit :: Lando, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zalun, Unassigned)

References

()

Details

(Keywords: conduit-triaged)

Attachments

(1 obsolete file)

``` lando-api_1 | File "/app/landoapi/api/landings.py", line 42, in land lando-api_1 | landing = Landing.create(revision_id, diff_id, api_key) lando-api_1 | File "/app/landoapi/models/landing.py", line 106, in create lando-api_1 | landing.create_patches(revisions, phab) lando-api_1 | File "/app/landoapi/models/landing.py", line 239, in create_patches lando-api_1 | parent_id).upload(phab).save() lando-api_1 | File "/app/landoapi/models/patch.py", line 86, in upload lando-api_1 | diff = phab.get_rawdiff(self.diff_id) lando-api_1 | File "/app/landoapi/phabricator_client.py", line 59, in get_rawdiff lando-api_1 | result = self._GET('/differential.getrawdiff', {'diffID': diff_id}) lando-api_1 | File "/app/landoapi/phabricator_client.py", line 206, in _GET lando-api_1 | return self._request(url, data, params, 'GET') lando-api_1 | File "/app/landoapi/phabricator_client.py", line 201, in _request lando-api_1 | raise exp lando-api_1 | landoapi.phabricator_client.PhabricatorAPIException: [Access Denied: Unknown Object (VOID)] (Can View) You do not have permission to view this object. // dkl (David Lawrence) can take this action. ```
Comment on attachment 8911144 [details] API endpoints react on PhabricatorAPIException Israel Madueme [:imadueme] has approved the revision. https://phabricator.services.mozilla.com/D78#1601
Attachment #8911144 - Flags: review+
Assignee: nobody → pzalewa
Component: General → Lando API
Comment on attachment 8911144 [details] API endpoints react on PhabricatorAPIException Israel Madueme [:imadueme] has been removed from the revision. https://phabricator.services.mozilla.com/D78#6983
Attachment #8911144 - Flags: review+
Comment on attachment 8911144 [details] API endpoints react on PhabricatorAPIException Israel Madueme [:imadueme] has approved the revision. https://phabricator.services.mozilla.com/D78
Attachment #8911144 - Flags: review+
Assignee: pzalewa → nobody
Is this closely related to bug 1436803?
Flags: needinfo?(smacleod)
(In reply to Mark Côté [:mcote] from comment #4) > Is this closely related to bug 1436803? Sort of related, but I'd say not technically part of the story. Without this we should still be returning an HTTP error response, which will be sent to the UI and displayed. The error itself might not be the most informative, but it will technically be sent. Improving the error messages is something I'd say is out of scope for the story, but should be done at some point.
Flags: needinfo?(smacleod)
Keywords: conduit-triaged
Whiteboard: [lando-backlog]
Attachment #8911144 - Attachment is obsolete: true
I think it should block the bug 1441728
I agree that this isn't ideal; however, we don't yet have proper support for landing confidential revisions, so I think this can wait until that story.
Blocks: 1443704
Whiteboard: [lando-backlog]
I think this bug is solved. The `PhabricatorAPIException` is caught and the 404 is returned. If we should provide more information - distinguish between not existing revision and lack of privileges, please reopen this bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: