(Error) Call to undefined method DifferentialChangesetQuery::withPHIDs()

Sometimes when viewing a diff, the user gets the following error in a popup:

[2020-04-29 13:35:37] EXCEPTION: (Error) Call to undefined method DifferentialChangesetQuery::withPHIDs() at [<phabricator>/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php:216]
arcanist(), phabricator(custom=7)
  #0 phlog(Error) called at [<phabricator>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]
  #1 PhabricatorDefaultRequestExceptionHandler::handleRequestThrowable(AphrontRequest, Error) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:752]
  #2 AphrontApplicationConfiguration::handleThrowable(Error) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:300]
  #3 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:211]
  #4 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:35]

It seems to be pretty random whether or not it happens, and it doesn’t reproduce for me. Some kind of caching bug?

Reproduction Instructions
I haven’t actually been able to reproduce it, but based on my logs it happens to people several times an hour.

Phabricator/Arcanist Version

Internal fork of Phabricator based on stable commit ce66b1c6287fc0983fa310caa0a80149ed772963

Based on the fact that getChangesetKey calls $changeset->getID(), I’m thinking the fix may just be to replace withPHIDs() with withIDs()?

diff --git a/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php b/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php
index a506ec38dc..1fa0d3e9fc 100644
--- a/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php
+++ b/src/infrastructure/diff/viewstate/PhabricatorChangesetViewStateEngine.php
@@ -213,7 +213,7 @@ final class PhabricatorChangesetViewStateEngine

         $other_changeset = id(new DifferentialChangesetQuery())
-          ->withPHIDs(array($other_phid))
+          ->withIDs(array($other_phid))

         $is_modified = false;

The reproduction steps are something like:

  • Create a revision with several diffs.
  • View Diff 1.
  • Hide a file.
  • View Diff 2, which affects the file you hid in Diff 1.

Your patch looks correct to me. I expect to upstream that or some near variation shortly.

https://secure.phabricator.com/T13519 has a note like “this logic might not actually work and should be verified”. :man_shrugging:

I believe this is fixed by https://secure.phabricator.com/D21189, which is now in master and stable. It’s effectively identical to your patch.

Thanks for the report!