I noticed this while testing the patch for T6757, but when you create a cyclic dependency, I no longer get a nice error dialog. Specifically, to reproduce, for any pair of tasks:
- Add T2 as a blocker of T1.
- Now, add T1 as a blocker of T2.
Expected behavior:
- Nice error dialog saying "you can't do that, you'd create a dependency cycle".
Actual behavior:
- Server-side unhandled exception.
Some notes:
- Relevant code is in PhabricatorSearchAttachController.
- We seem to throw PhabricatorEdgeCycleException correctly. This raises out of an EdgeEditor inside the TransactionEditor which does most of the work.
- The controller has code to convert this into a human-readable message, in raiseGraphCycleException(), but it is never called.
Some approaches might be:
- Use blame to track down what removed raiseGraphCycleException() and see what it used to do, then do that if it's easy.
- Use return $this->newDialog(...) to just return a dialog explicitly.
- (Probably overreaching) Improve top-level exception handling? I think we used to get some kind of dialog back on unhandled exceptions. This might be configuration-dependent. Sending anything back would be significantly better than the current behavior. Providing a PhabricatorUsageException($message) or similar -- which serves as a hint that the message is user-friendly and we can always shows the message details, regardless of configuration settings -- might be a step up from that. This might get resolved by, e.g., T6692 before this gets tackled, and provide a more straightforward approach.
