Page MenuHomePhabricator

When query panels overheat, degrade them and show a warning
ClosedPublic

Authored by epriestley on Wed, Mar 27, 3:45 PM.

Details

Summary

Depends on D20334. Ref T13272. After recent changes to make overheating queries throw by default, dashboard panels now fail into an error state when they overheat.

This is a big step up from the hard-coded homepage panels removed by D20333, but can be improved. Let these panels render partial results when they overheat and show a human-readable warning.

Test Plan

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Wed, Mar 27, 3:45 PM
epriestley requested review of this revision.Wed, Mar 27, 3:47 PM
amckinley accepted this revision.Wed, Mar 27, 9:31 PM
amckinley added inline comments.
src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php
120–128

I think this text exposes a little more implementation detail than users want to know about? In both cases I'd remove the explanation and change them respectively to:

Your query took too long, so only some results are shown. Refine your query to find more results.

Your query took too long and gave up before finding any results. Refine your query to find results.

This revision is now accepted and ready to land.Wed, Mar 27, 9:31 PM

I think knowing how to refine the query might be helpful, especially for more technical users. In particular, the hint "too many results are not visible to you" implies the correct remedy ("change the query to match only stuff I can see"), at least if you're paying close attention. The hint "took too long" might imply the wrong remedy ("simplify the query"). Simplifying the query by removing constraints tends make overheating worse by matching more stuff -- you should usually add constraints, making the query more complex but excluding results you can't see.

I think knowing how to refine the query might be helpful, especially for more technical users.

That's fair. I just think users aren't used to thinking of queries as having distinct "fetching" and "policy enforcement" phases. Also, it feels kind of vaguely user-hostile to tell someone that "most of these objects aren't visible to you", like finding out you've been given the kiddie wine list at a nice restaurant. Maybe "Filtering your query results is taking too long. To see more results, speed up filtering by adding additional constraints." or similar? Not a big deal; I'm ok with this if you like it as-is.

I do generally agree this flow isn't great. I'm not sure we can produce a good outcome for less-technical users: if we don't explain that this is a policy filtering issue, I think even technical users are unlikely to be able to guess how to refine the query, but if we do explain that this is a policy issue it turns into this paragraph of very technical details that necessarily says "you can't see some of the results". (I'm not sure if "not visible to you" or "you don't have permission" is more of a kid-menu phrasing and can't immediately come up with a better one.)

Possibly we should just accept a friendly-but-useless error like "OwO this took too long! we're very sowwy!" under the assumption that no user is likely to be able to navigate this under any reasonable error message. Or "OwO sowssy wittle qwewy oopsie! OuO view very technical detaiws (experts only!) OuO". That probably looks more like this:

Query Overheated
This query took too long, so only some results are shown. Learn More

We do already have similar language in ApplicationSearchController->newOverheatedView() and it hasn't caused problems yet, for whatever that's worth.

  • Try the "OwO oopsie woopsie Learn More" route.
  • Unify the message in panels and main search.
This revision was automatically updated to reflect the committed changes.