Enforce \Exception instances as rejection values

[jsor]

This also removes support for rejecting with a Promise instance.

Decide whether the rejection value is still an optional argument and can be null. Enforcing an argument is more inline with throw (you can't throw null) and let's us remove UnhandledRejectionException.

[daltones]

+1 👍

[clue]

👍

[joshdifabio]

Note that some() currently rejects with an array of promises. Maybe we need some kind of composite exception class.

[jsor]

Good catch, thanks 👍

[joshdifabio]

I think you should allow \Throwable in PHP7.

public function __construct($e)
{
  if (PHP_MAJOR_VERSION < 7) {
    if (!$e instanceof \Exception) {
      throw new \InvalidArgumentException;
    }
  } elseif (!$e instanceof \Throwable) {
    throw new \InvalidArgumentException;
  }
}

[jsor]

Another good catch 👍

[khelle]

Creating exceptions is very heavy for PHP due to filling them with data on creation and not on throwing. Just check what happens when you create 10k of promises, and reject them all. In best sitatuion you get MemoryOverflow error, in worst core dumped. It is reasonable to allow rejection with different values than Throwable for this purpose. I think the good idea would be to allow string or Throwable or some lazy Throwable object defined inside library.

[codedokode]

@joshdifabio

Note that some() currently rejects with an array of promises. Maybe we need some kind of composite exception class.

Maybe we could solve this if some() would reject with the first exception, and not wait for other values? And make a separate function returning a promise that resolves into an array of rejected values.

I think a rejected value should always be something Throwable and there would be no arrays.

[codedokode]

@khelle

I think you might be mistaking about memory consumption, I ran the following code:

php -r '$e = []; for ($i = 0; $i < 10000; $i++) { $e[] = new Exception("Test"); }; var_dump(memory_get_usage()); '

And I get 4-5Mb of used memory (that is 500 bytes per Exception object). Maybe you were using some xdebug options that add additional information into an exception?

I also tried this code:

use React\Promise\Deferred;

$p = [];
for ($i=0; $i < 10000; $i++) { 
    $p[] = new Deferred();
}

foreach ($p as $deferred) {
    $deferred->reject(new \Exception("Test"));
}

var_dump(memory_get_peak_usage());

It prints 20 Mb peak memory usage (2 Kb per rejected promise). So it is not exceptions that consume memory but something else. Promises create a lot of anonymous functions inside, maybe that could be the reason.

[khelle]

@codedokode This is known mistake of PHP that it popullates Exception on creation and not on throwing like in other programming languages, for example in JAVA. There is no dyning that. The example you provided without using exceptions would take about ~10 Mb peak memory, meaning the exception took 10 Mb. It is already high enough for exceptions that literally have empty stack trace. Now try the same example in real world application, where the thrown exception would have about ~10-20 elements in their stack trace. You will run out of memory.