[00:02:19] Amir1: what kind of errors is this meant to respond to? It seems odd to incorporate fatal errors into business logic. [00:03:04] at least I'd expect an inline comment indicating what kind of fatal errors we have seen in the past in this kind of scenario, to allow for refactoring toward something less indirect in the future. [00:05:30] Amir1: I believe the diff in that comment did not yet end up applied, is that right? [04:12:02] Krinkle: oh this is for handling validation errors mostly, if you send property value with the wrong datatype, you'd be wasting a Q-id without saving it [04:12:18] not fatals or db connection issues [14:58:54] Amir1: hmm okay. Maybe onException isn't what I thought it was then [15:00:04] I double checked and anything returning "error" (including validation) trigger this. [15:00:14] but I don't like registering a hook on the fly [15:00:30] checking codebase it's only used in tests [15:21:51] Amir1: Validation sounds like you mean API errors (e.g. 'error' => { code: 'badparam' } and such), but afaik onException really is about thrown exceptions. [15:22:06] I thought such error messages are not usually done in api.php through exceptions? [15:22:24] and indeed it seems like this would (also, or only) get called for uncaught exceptions [15:22:52] I double check but I think it gets triggered in error [15:22:53] If the thing you're looking for really happens to be an exception internally, then I suppose maybe a try-catch somewhere in the wikibase api subclass would work? [15:23:00] If not, I need to find another solution [15:24:23] OK, I think we're both right. [15:24:31] I didn't realize dieWithError() throws ApiUsageException [15:24:44] although it says to avoid throwing it directly, it is indeed internally based on an excepetion. [15:26:15] So a subset of API onException will be non-fatal normal api client "error" responses [15:27:12] But if those come from your own code, then maybe there's a more direct way you can handle this, e.g. have the code that sets that error code also do the increment, or maybe some kind of method this api class can override and use to do the increment etc. [15:27:39] I thought the reason you (plural) were considering this approach is because they were meant to cover fatal errors that you can't control. [15:43:03] I think fatals would be a very small portion of the calls to that hook handler [15:43:30] > I didn't realize dieWithError() throws ApiUsageException [15:43:31] That's how I came to conclusion that this hook gets triggered [17:54:07] James_F: I imagine the hhvm-php72 layers in icu/uppercasing should probably settle before the next php upgrade. [17:54:11] or did we finish that already? [18:03:25] Krinkle: It'd be nice if that was done, yes. Not yet complete, AFAIAA. An Editing/ServiceOps thing at this point? [18:04:28] yeah, I suppose.