Page MenuHomePhabricator

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

Authored by epriestley on Mar 16 2018, 8:26 PM.
Tags
None
Referenced Files
F13061529: D19232.diff
Fri, Apr 19, 7:33 PM
Unknown Object (File)
Thu, Apr 11, 9:07 AM
Unknown Object (File)
Sat, Apr 6, 4:11 AM
Unknown Object (File)
Sun, Mar 31, 11:23 AM
Unknown Object (File)
Sun, Mar 31, 2:33 AM
Unknown Object (File)
Sun, Mar 31, 2:33 AM
Unknown Object (File)
Sun, Mar 31, 2:33 AM
Unknown Object (File)
Sat, Mar 30, 6:12 AM
Subscribers
Restricted Owners Package

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a subscriber: Restricted Owners Package.Mar 16 2018, 8:26 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 16 2018, 8:27 PM
epriestley requested review of this revision.
This revision was automatically updated to reflect the committed changes.