Adjudicator import attempts to access ID of non-object after creation failure

[czlee]

This happened:

PHP Notice – yii\base\ErrorException Trying to get property of non-object in /var/www/virtual/tabbie/tabbie2/tabbie2.git/frontend/controllers/AdjudicatorController.php at line 362

    if (count($row[1]) == 1) { //NEW
        $userA = User::NewViaImport($row[1][0], $row[2][0], $row[3][0], $societyID, !$model->is_test, $tournament);
        $userAID = $userA->id;           // <------------ crashes at this line ------------
    } else if (count($row[1]) == 2) {
        $userAID = $row[1][1]["id"];
    } else {

The original failure was a user data error—I checked my import file again after the error and there was a trailing comma in the e-mail address of the offending line (which inferred from the $_SESSION variable on the debug page). I notice that User::NewViaImport returns false if there was a failure to create the object. While this occurs as a result of bad input, probably one might like it to fail a bit more gracefully, ideally explaining the original error if possible.

This also happened to me with importing teams (with the same root cause user error) but in that case it just showed a plain-text "internal server error" page without debug information, and continued to import all the other teams (even those that came after the error). In the adjudicators case, because it crashes immediately, all adjudicators from the offending line are not imported.

[chief-nerd]

Is this still a thing?

[rscoates]

I've never had this problem

[czlee]

I just reproduced it, it's now line 381 of the same file. To reproduce it, copy this into a new CSV file:

Society,First Name,Last Name,Email,Strength
Swing,Test,Account,"invalid@email,",70

then try importing it under "Import Adjudicators". The confirmation screen will show up fine. However, clicking "Make it so" on the next page will cause the exception, and show the page with debugging information.

As I say, this is a user data error—you'll notice the e-mail address is invalid, so it fails model rule validation. But probably one might like it to fail a bit more gracefully than an uncaught exception. The error message at User.php:408 does register, but because of the uncaught exception, it only gets displayed to the user on the first valid page loaded after the debug page.

[rscoates]

Indeed. That seems very reasonable.

Best Wishes,

Richard Coates

On 29 May 2017, at 10:36, Chuan-Zheng Lee [email protected] wrote:

I just reproduced it, it's now line 381 of the same file. To reproduce it, copy this into a new CSV file:

Society,First Name,Last Name,Email,Strength Swing,Test,Account,"invalid@email,",70 then try importing it under "Import Adjudicators". The confirmation screen will show up fine. However, clicking "Make it so" on the next page will cause the exception, and show the page with debugging information.

As I say, this is a user data error—you'll notice the e-mail address is invalid, so it fails model rule validation. But probably one might like it to fail a bit more gracefully than an uncaught exception. The error message at User.php:408 does register, but because of the uncaught exception, it only gets displayed to the user on the first valid page loaded after the debug page.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.