Page MenuHomePhabricator

Allow dashboard panels to detect rendering cycles and arrest stack overflows
ClosedPublic

Authored by epriestley on May 15 2014, 6:54 PM.
Tags
None
Referenced Files
F14478338: D9140.diff
Sat, Dec 28, 5:19 PM
Unknown Object (File)
Tue, Dec 24, 6:47 AM
Unknown Object (File)
Sun, Dec 8, 4:50 PM
Unknown Object (File)
Sat, Dec 7, 7:35 AM
Unknown Object (File)
Thu, Dec 5, 4:08 AM
Unknown Object (File)
Thu, Dec 5, 3:56 AM
Unknown Object (File)
Sun, Dec 1, 5:28 AM
Unknown Object (File)
Nov 27 2024, 6:23 AM
Subscribers

Details

Summary

Ref T4986. Ref T4983. Panels will soon be able to contain other panels, either via Remarkup ({W1}) or maybe through new types of meta-panels.

Allow panels to detect that they are being rendered very deeply and/or within themselves.

Test Plan

Faked some errors, got failed panel renders. Since panels can't really contain other panels yet, this doesn't really have an impact.

Diff Detail

Repository
rP Phabricator
Branch
dash5
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 491
Build 491: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Allow dashboard panels to detect rendering cycles and arrest stack overflows.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
btrahan added inline comments.
src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php
18

I'd expect this to be some class that extends the Aphront Graph class with cycle detection. (To support panel A which contains panel B which contains panel C, etc.) You'd probably want to check the graph out during edits at the dashboard / panel level and otherwise not load it.

120

hahaha, tip jar time

src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php
28

this will be an error i think

This revision is now accepted and ready to land.May 15 2014, 8:21 PM
src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php
18

I think we probably won't be able to guarantee this in general. For example, suppose you have a panel (W123) showing the 5 latest feed stories, and we update feed so it renders remarkup, then someone makes a comment with {W123} in it. We can't (and shouldn't) block the comment.

We could do this in a more narrow way, but I suspect there will be so many cases like this that making any effort isn't worth the complexity, and just failing gracefully is good enough without trying to prevent things from getting into this state.

src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php
28

Haha, good catch. This was test code that I neglected to remove.

epriestley updated this revision to Diff 21727.

Closed by commit rP63acd90cefaf (authored by @epriestley).