@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Add more type checking to transactions queued by Herald
Summary:
See PHI1096. Depends on D20213. An install is reporting a hard-to-reproduce issue where a non-transaction gets queued by Herald somehow. This might be in third-party code.
Sprinkle the relevant parts of the code with `final` and type checking to try to catch the problem before it causes a fatal we can't pull a stack trace out of.
Test Plan: Poked around locally (e.g., edited revisions to cause Herald to trigger), but hard to know if this will do what it's supposed to or not without deploying and seeing if it catches anything.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D20214
+24
-15
-9
src/applications/differential/editor/DifferentialDiffEditor.php
-9
src/applications/differential/editor/DifferentialDiffEditor.php
···
208
208
return $adapter;
209
209
}
210
210
211
-
protected function didApplyHeraldRules(
212
-
PhabricatorLiskDAO $object,
213
-
HeraldAdapter $adapter,
214
-
HeraldTranscript $transcript) {
215
-
216
-
$xactions = array();
217
-
return $xactions;
218
-
}
219
-
220
211
private function updateDiffFromDict(DifferentialDiff $diff, $dict) {
221
212
$diff
222
213
->setSourcePath(idx($dict, 'sourcePath'))
+17
-4
src/applications/herald/adapter/HeraldAdapter.php
+17
-4
src/applications/herald/adapter/HeraldAdapter.php
···
186
186
return $this->appliedTransactions;
187
187
}
188
188
189
-
public function queueTransaction($transaction) {
189
+
final public function queueTransaction(
190
+
PhabricatorApplicationTransaction $transaction) {
190
191
$this->queuedTransactions[] = $transaction;
191
192
}
192
193
193
-
public function getQueuedTransactions() {
194
+
final public function getQueuedTransactions() {
194
195
return $this->queuedTransactions;
195
196
}
196
197
197
-
public function newTransaction() {
198
+
final public function newTransaction() {
198
199
$object = $this->newObject();
199
200
200
201
if (!($object instanceof PhabricatorApplicationTransactionInterface)) {
···
205
206
'PhabricatorApplicationTransactionInterface'));
206
207
}
207
208
208
-
return $object->getApplicationTransactionTemplate();
209
+
$xaction = $object->getApplicationTransactionTemplate();
210
+
211
+
if (!($xaction instanceof PhabricatorApplicationTransaction)) {
212
+
throw new Exception(
213
+
pht(
214
+
'Expected object (of class "%s") to return a transaction template '.
215
+
'(of class "%s"), but it returned something else ("%s").',
216
+
get_class($object),
217
+
'PhabricatorApplicationTransaction',
218
+
phutil_describe_type($xaction)));
219
+
}
220
+
221
+
return $xaction;
209
222
}
210
223
211
224
+7
-2
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+7
-2
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
···
3779
3779
3780
3780
$this->mustEncrypt = $adapter->getMustEncryptReasons();
3781
3781
3782
+
$apply_xactions = $this->didApplyHeraldRules($object, $adapter, $xscript);
3783
+
assert_instances_of($apply_xactions, 'PhabricatorApplicationTransaction');
3784
+
3785
+
$queue_xactions = $adapter->getQueuedTransactions();
3786
+
3782
3787
return array_merge(
3783
-
$this->didApplyHeraldRules($object, $adapter, $xscript),
3784
-
$adapter->getQueuedTransactions());
3788
+
array_values($apply_xactions),
3789
+
array_values($queue_xactions));
3785
3790
}
3786
3791
3787
3792
protected function didApplyHeraldRules(