Thanks my friend

I have some doubts about the root problem here. Maybe it's OK that we set NULL from application transactions.

Looking at the stack trace, this also seems interestingly wrong, and maybe it's the only thing that we should handle:

public function setValueFromStorage($value) {
  // NOTE: We're accepting either a JSON string (a real storage value) or
  // an array (from HTTP parameter prefilling). This is a little hacky, but
  // should hold until this can get cleaned up more thoroughly.
  // TODO: Clean this up.

  $result = array();
  if (!is_array($value)) {  // ←←←←←←←←← May be this needs to be converted to is_string($value) or similar thing
    $value = json_decode($value, true);
    if (is_array($value)) {
      $result = array_values($value);
    }
  }

  $this->setFieldValue($value);

  return $this;
}

This seems reasonable since it's nonsense to me to decode an integer, or decode a float. Only decoding a string makes sense to me.

What do we think about?

What do you think about? Maybe I can try to propose a counter-patch about the related tip

In D25390#11765, @valerio.bozzolan wrote:

if (!is_array($value)) {  // ←←←←←←←←← May be this needs to be converted to is_string($value) or similar thing

This seems reasonable since it's nonsense to me to decode an integer, or decode a float. Only decoding a string makes sense to me.

Yes this sounds like a cleaner approach indeed so I updated the patch.
However I realize I just patched that very line in rPe610e739cb4294dcab92c3145285a5ffa5c3cf61, so had to adjust a bit more.

Testing both T15602 and T15683 by having both a select and users type custom field configured in /config/edit/maniphest.custom-field-definitions/ I get no exceptions or warnings anymore.

I'm still puzzled by the unused $result variable in line 46.

The code setting $result does get reached when: having a custom field which takes more than a single value defined like in T15602 && a Maniphest form configured which uses that custom field, e.g. going to http://phorge.localhost/maniphest/task/edit/form/4/?parent=1&status=open&custom.train.backup=PHID-USER-c33pgqltkiv6v6yl6sz2,PHID-USER-4wojqm5n3xc3qmpzso4z&title=fooo (note the custom.train.backup) && then creating that task. Maybe someone wants to experiment more with these hints, or has an idea what the original intention of an additional call to array_values() here could have been:

if (is_string($value) && phutil_nonempty_string($value)) {
  phlog(pht($value));                   // example output:  ["PHID-USER-c33pgqltkiv6v6yl6sz2","PHID-USER-4wojqm5n3xc3qmpzso4z"]
  $value = json_decode($value, true);
  if (is_array($value)) {               // this is true for our $value because of the json_decode(...,true) called in the line before
    $result = array_values($value);
    phlog(pht(implode(",", $result)));  // example output:  PHID-USER-c33pgqltkiv6v6yl6sz2,PHID-USER-4wojqm5n3xc3qmpzso4z
  }
}
$this->setFieldValue($value);
return $this;

I propose to remove that unused $result array. It's been unused before and looks we were not aware of any issues created by that fact.