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)
Conduit
Lando
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 1•7 years ago
|
||
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+
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → pzalewa
Component: General → Lando API
Comment 2•7 years ago
|
||
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 3•7 years ago
|
||
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+
Reporter | ||
Updated•7 years ago
|
Assignee: pzalewa → nobody
Comment 4•7 years ago
|
||
Is this closely related to bug 1436803?
Flags: needinfo?(smacleod)
Keywords: conduit-needs-discussion
Comment 5•7 years ago
|
||
(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)
Updated•7 years ago
|
Keywords: conduit-triaged
Whiteboard: [lando-backlog]
Updated•7 years ago
|
Attachment #8911144 -
Attachment is obsolete: true
Reporter | ||
Comment 6•7 years ago
|
||
I think it should block the bug 1441728
Comment 7•7 years ago
|
||
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]
Reporter | ||
Comment 8•7 years ago
|
||
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.
Description
•