Details
- Reviewers
epriestley - Maniphest Tasks
- T6419: Error for `metamta.diffusion.byte-limit` does not point at config option
- Commits
- Restricted Diffusion Commit
rPdd49096a857b: Diffusion - upgrade exception error message to include hint to config option
looks nice
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php | ||
---|---|---|
62–66 | ye old try and catch around this code |
src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php | ||
---|---|---|
64–65 | Maybe we should just remove this phlog() -- it's obvious from the mail what happened, and some users tend to find it confusing when we log expected/recovered exceptions (T6406 is similar -- the error seems scary but is extremely minor). Either that, or add something like phlog_handled_exception() and have it print some generic "A minor error occurred / not a big deal / we are continuing normally / no cause for alarm" thing. But I think there's almost no value in logging this at all. Another high-effort alternative would be to add a verbose flag to Workers. Currently, daemons have a verbose mode which controls whether $this->log() goes anywhere. Workers have a $this->log(), but it's always active. We could give workers a verbose flag, have them inherit it from the daemon when run from the taskmaster, and set it on a case-by-case basis in other contexts, then call $this->log('%s', $ex->getMessage()) instead of phlog(). But just removing this line seems clean and way easier. |