Trying to submit a diff via arc diff. Getting the following error:
Exception ERR-CONDUIT-CORE: Argument 2 passed to HeraldDifferentialRevisionAdapter::newLegacyAdapter() must be an instance of DifferentialDiff, null given, called in /core/lib/phabricator/src/applications/differential/editor/DifferentialTransactionEditor.php on line 1585 and defined
| rP Phabricator | |||
| D17139 | rPef05bf335d8e Allow Harbormaster builds to publish to a different object | ||
| D13441 | rPdb1bc7fd7f1f Carefully avoid the harbormaster/differential race | ||
| rP520e70a3cf4c Fix a bad method call which slipped by | |||
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Resolved | epriestley | T8089 Unprototype Harbormaster (v1) | ||
| Resolved | None | T8657 Harbormaster v1 Errata | ||
| Resolved | epriestley | T8650 DifferentialTransactionEditor may race HarbormasterBuildWorker |
I think this is a race between the HarbormasterBuildWorker queued by harbormaster.sendmessage, and the DifferentialTransactionEditor attaching the diff to the revision.
Specifically, both of them may update the DifferentialDiff object:
If they race correctly, we'll end up with the BuildWorker overwriting the revisionID property and setting it back to null. This causes the active diff to fail to load later on. This being a race is consistent with the sporadic nature of the error.
Several possible approaches spring to mind here:
I'm tentatively going to do something in the vein of (2) to stop this from bleeding and pursue a more complete fix after cutting stable. We have a general issue with this code not making sense:
So this needs to be more nuanced anyway. Publishing "pass" for autoplans isn't helpful and publishing to the diff isn't helpful.
@epriestley Can we get a DB migration that cleans up all of the bad revisions created by this bug? Apparently it affected a bunch of people and now there are revisions they can't remove from their queue (I can do bin/destroy manually, but that relies on people telling me all of the revision numbers).
SELECT CONCAT('D', r.id) FROM differential_revision r
LEFT JOIN differential_diff d
ON d.revisionID = r.id
WHERE d.revisionID IS NULL;The revision editor and diff editor should no longer be racing to edit the diff. The two revision editors may be racing now, but that race is significantly easier to resolve if it crops up.