There are, in total, 3 call-sites to setErrorListener - in for the daemons, one for dark-console, and one (PhutilErrorLog) that lives in arcanist but is only used by the SSHD scripts.

Notably, that means the the listener function isn't used when running arc, and probably also in any of the scripts in phorge/bin/.

lint + a typofix

Harbormaster completed remote builds in B747: Diff 1242.Aug 12 2023, 06:40

avivey planned changes to this revision.Aug 12 2023, 06:51

avivey mentioned this in T15554: Handling PHP deprecations: convert to Setup Issues.Aug 12 2023, 17:11

avivey mentioned this in D25440: Show Deprecation Warnings as Setup Warnings.Sep 22 2023, 19:41

Sten subscribed.Sep 23 2023, 11:16

valerio.bozzolan added inline comments.Oct 11 2023, 05:30

src/workflow/ArcanistLookWorkflow.php
57 ↗	(On Diff #1242)	Probably this is a test in production

avivey edited the test plan for this revision. (Show Details)Nov 24 2023, 17:10

cleanup

Harbormaster completed remote builds in B949: Diff 1536.Nov 24 2023, 17:13

avivey mentioned this in Z1: Phorge.Nov 24 2023, 18:28

I like renaming the function!

We should probably have a timeline for removing the old function. Do we have a task for that?

src/filesystem/PhutilErrorLog.php
31 ↗	(On Diff #1536)	The linter did point out a legitimate concern here, however; this doesn't seem to be your code change. Should we file a new task?

This revision is now accepted and ready to land.Nov 26 2023, 06:57

In D25388#13843, @Matthew wrote:

I like renaming the function!

We should probably have a timeline for removing the old function. Do we have a task for that?

I feel like that kind of task will just fall down into oblivion....

src/filesystem/PhutilErrorLog.php
31 ↗	(On Diff #1536)	done - T15677

In D25388#13885, @avivey wrote:

In D25388#13843, @Matthew wrote:

I like renaming the function!

We should probably have a timeline for removing the old function. Do we have a task for that?

I feel like that kind of task will just fall down into oblivion....

I agree, but that the alternative is leaving deprecated code around. I think we should probably come up with a formal deprecation plan, but that seems out of the scope here. I'll think on it and write up a task :)

This revision was landed with ongoing or failed builds.Nov 27 2023, 18:32

Closed by commit rARC25611ba24add: PhutilErrorHandler: support multiple error listeners. · Explain Why

This revision was automatically updated to reflect the committed changes.

avivey added a commit: rARC25611ba24add: PhutilErrorHandler: support multiple error listeners.

Sorry for the delayed review

src/error/PhutilErrorHandler.php
458	Is this something that should have try/catch around the invocation here? So one error handler doesn't impact the others from running?

avivey added inline comments.Dec 1 2023, 09:05

src/error/PhutilErrorHandler.php
458	Yeah, good idea. I'll try to diff something.

avivey mentioned this in rP2ba2cbaf9bbf: Show Deprecation Warnings as Setup Warnings.Dec 8 2023, 15:56

avivey mentioned this in T15690: PhutilErrorHandler - add try/catch around `call_user_func`.Dec 8 2023, 16:04

avivey added inline comments.

src/error/PhutilErrorHandler.php
458	Filed T15690, because I'm not sure how to handle that right now...

avivey mentioned this in Q100: Undefined method when using celelery map on commit 2295bcda (Answer 127).Dec 12 2023, 19:31

speck added inline comments.Jun 22 2024, 03:29

src/error/PhutilErrorHandler.php
78	How many callers are there? Can we make updates so this can be removed? Or is this something commonly used via extensions?

avivey added inline comments.Jun 22 2024, 15:44

src/error/PhutilErrorHandler.php
78	It's not common to call this, but there might be some extensions that use it to integrate with some error reporting. We can remove it whenever we feel like.