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.