1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-22 19:49:02 +01:00

Reduce the severity of policy fatals when building the Harbormaster "build status" element

Summary:
See PHI430. Ref T13102. When the "Build Status" element raises a policy exception, we currently fatal the whole page rather than raising a normal policy error.

This is because the policy check happens very late in page construction, long after we've made the decision to show the page instead of a policy error, and gets treated as a rendering error.

In turn, this is because the rendering is event-based rather than using a more modern Engine + EngineExtension sort of construct, so some of the actual logic runs way later than it should.

Since unwinding all of this isn't trivial and the current behavior is materially bad, limit the damage here for now by just hiding the element. See T13088 for notes on handling this in a more nuanced way in the future.

Test Plan:
  - Created a revision visible to "Public".
  - Ran a build against it with a build plan visible to "All Users".
  - Viewed revision in an incognito window.
    - Before patch: Policy fatal with a red "rendering phase" error box.
    - After patch: Mostly-functional page with a missing "Build Status" element.
  - Viewed revision as a user with a normal session, saw the same UI before and after the change.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19232
This commit is contained in:
epriestley 2018-03-16 13:18:35 -07:00
parent 667955b8ae
commit fa6cd200e8

View file

@ -51,13 +51,22 @@ final class HarbormasterUIEventListener
return;
}
$buildable = id(new HarbormasterBuildableQuery())
->setViewer($viewer)
->withManualBuildables(false)
->withBuildablePHIDs(array($buildable_phid))
->needBuilds(true)
->needTargets(true)
->executeOne();
try {
$buildable = id(new HarbormasterBuildableQuery())
->setViewer($viewer)
->withManualBuildables(false)
->withBuildablePHIDs(array($buildable_phid))
->needBuilds(true)
->needTargets(true)
->executeOne();
} catch (PhabricatorPolicyException $ex) {
// TODO: See PHI430. When this query raises a policy exception, it
// fatals the whole page because it happens very late in execution,
// during final page rendering. If the viewer can't see the buildable or
// some of the builds, just hide this element for now.
return;
}
if (!$buildable) {
return;
}