Page MenuHomePhorge

RuntimeException: "Undefined index" when left diff does not exist in DifferentialRevisionTimelineEngine.php
Open, Needs TriagePublic

Description

Go to https://we.phorge.it/transactions/showolder/PHID-DREV-em7vleytcec4vifhqir5/?after=16576&quoteTargetID=UQ0_1&quoteRef=D25576

Backtrace from the same issue in downstream Wikimedia instance in 2023:

EXCEPTION: (RuntimeException) Undefined index:  at [<arcanist>/src/error/PhutilErrorHandler.php:251]
 arcanist(), ava(), phorge(), translations(), wmf-ext-misc()
   #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phorge>/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php:24]
   #1 <#2> DifferentialRevisionTimelineEngine::newTimelineView() called at [<phorge>/src/applications/transactions/engine/PhabricatorTimelineEngine.php:70]
   #2 <#2> PhabricatorTimelineEngine::buildTimelineView() called at [<phorge>/src/applications/base/controller/PhabricatorController.php:524]
   #3 <#2> PhabricatorController::buildTransactionTimeline(DifferentialRevision, DifferentialTransactionQuery, NULL, array) called at [<phorge>/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php:38]
   #4 <#2> PhabricatorApplicationTransactionShowOlderController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
   #5 phlog(RuntimeException) called at [<phorge>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]
   #6 PhabricatorDefaultRequestExceptionHandler::handleRequestThrowable(AphrontRequest, RuntimeException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
   #7 AphrontApplicationConfiguration::handleThrowable(RuntimeException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
   #8 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:204]
   #9 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

Related code is

$view_data = $this->getViewData();
if (!$view_data) {
  $view_data = array();
}

$left = idx($view_data, 'left');
$right = idx($view_data, 'right');

$diffs = id(new DifferentialDiffQuery())
  ->setViewer($viewer)
  ->withIDs(array($left, $right))
  ->execute();
$diffs = mpull($diffs, null, 'getID');
$left_diff = $diffs[$left];     // <- line that throws the exception
$right_diff = $diffs[$right];

Obviously $view_data can be null as there is a check to set it to an empty array; idx does not have a default return value defined.

Related Objects

Event Timeline

I'm tempted to just change that one line to $left = idx($view_data, 'left', 0); so there shoud not be a crash at this stage anymore but instead the default value 0 is passed to the DifferentialDiffQuery and to the new DifferentialTransactionView... but I lack a reproducer to check if all other code is fine with this.

I cannot reproduce this. Can you?

I can only show a correct warning "This file was added." on the left, if left is missing.

As Wikimedia uninstalled Differential I cannot further debug in downstream.
We can either decline the ticket and its patch for now (if someone runs into this again, they could reopen or file a new task), or could get the patch in (setting a default value) without a test plan to have more robust code. Shrug.
Opinions? :)

Abandoning per last comment

Seems to happen in upstream per duplicate, thus reopening