No Error Handling for forbidden socket connections / Amp should be optional

[c33s]

Question	Answer
Box version	3.0.0-alpha.3
PHP version	7.1.16
Platform with version	Win7x64
Github Repo	-

running box on a firewalled windows system where all connections are blocked by default for all processes. if i run box build i get screens of error messages (i try to put a symfony4 skeleton in a phar file).

[ERROR] _HumbugBox5adef95163555\Amp\Process\ProcessException: Failed to connect socket #0: 10013: An attempt was made
         to access a socket in a way forbidden by its access permissions. in
         phar://C:/bin/box.phar/vendor/amphp/process/lib/Internal/Windows/SocketConnector.php:258
Stack trace:

if i allow the php process to create a connection the errors are gone and box is building. in comparision to box 2 the amp part has the opposite of a speed increase. what box2 has build in 63sec box3 was not able to do in over 30min. had to terminate the process, the firewall was configured to allowed php to access the local dynamic port and also the temp file ampCB1B.tmp was allowed to connect.

amp is a nice concept but it should be really optional, it has quite many side effects.

having to allow dynamic port access which i can't control was never i concept i liked. also having files in the temp dir, which are not firewalled is really uncool (need dynamic access). additional i have to bring my firewall in dialog mode every time i want to run box because the temp file is generated with a random name, so i have to create new rules for each file at each run.

please add a old school non-amp processing.

box.json.dist

files:
   - composer.json
directories:
   - bin
   - config
   - src
   - lib
finder:
   -
     name: '*.*'
     in: vendor
     exclude:
       - .gitignore
       - .md
       - phpunit
       - Tester
       - Tests
       - tests
compactors:
   - Herrera\Box\Compactor\Json
   - Herrera\Box\Compactor\Php
compression: GZ
main: bin/puppet-enc
output: build/puppet-enc.phar
stub: true
git-commit: git_commit
git-version: git_version
git-tag: git_tag
chmod: '0755'

Output

box build
converting to json


[WARNING] The command "build" is deprecated. Use "compile" instead.



   ____
  / __ )____  _  __
 / __  / __ \| |/_/
/ /_/ / /_/ />  <
/_____/\____/_/|_|


Box version 3.0.0-alpha.3 build 6b7dc4a883c199c93403adbc7dfaefdd52fc9336


// Loading the configuration file
// "box.json".

Building the PHAR "./build/puppet-enc.phar"
? Setting replacement values
 + @git_commit@: 5f620495beb07c9304e4bd88dafa11e45224b1c5
 + @git_tag@: 1.0.1
 + @git_version@: 1.0.1
? Registering compactors
 + KevinGH\Box\Compactor\Json
 + KevinGH\Box\Compactor\Php
? Adding main file: ./bin/puppet-enc
? Adding requirements checker
? Adding binary files
   > No file found
? Adding files


[ERROR] _HumbugBox5adef95163555\Amp\Process\ProcessException: Failed to connect socket #0: 10013: An attempt was made
        to access a socket in a way forbidden by its access permissions. in
        phar:///bin/box.phar/vendor/amphp/process/lib/Internal/Windows/SocketConnector.php:258
Stack trace:
#0
        phar:///bin/box.phar/vendor/amphp/amp/lib/Loop/NativeDriver.php(91):
        _HumbugBox5adef95163555\Amp\Process\Internal\Windows\SocketConnector->onProcessConnectTimeout('bo',
        Object(_HumbugBox5adef95163555\Amp\Process\Internal\Windows\Handle))
#1
        phar:///bin/box.phar/vendor/amphp/amp/lib/Loop/Driver.php(121):
        _HumbugBox5adef95163555\Amp\Loop\NativeDriver->dispatch(true)
#2
        phar:///bin/box.phar/vendor/amphp/amp/lib/Loop/Driver.php(69):
        _HumbugBox5adef95163555\Amp\Loop\Driver->tick()
#3
        phar:///bin/box.phar/vendor/amphp/amp/lib/Loop.php(76):
        _HumbugBox5adef95163555\Amp\Loop\Driver->run()
#4
        phar:///bin/box.phar/vendor/amphp/amp/lib/functions.php(151):
        _HumbugBox5adef95163555\Amp\Loop::run(Object(Closure))
#5 phar:///bin/box.phar/src/Box.php(264):
        _HumbugBox5adef95163555\Amp\Promise\wait(Object(_HumbugBox5adef95163555\Amp\Coroutine))
#6
        phar:///bin/box.phar/src/Box.php(153): _HumbugBox5adef95163555\KevinGH\Box\Box->processContents(Array,
        '...')
#7 phar:///bin/box.phar/src/Console/Command/Compile.php(222):
        _HumbugBox5adef95163555\KevinGH\Box\Box->addFiles(Array, false, true)
#8
        phar:///bin/box.phar/src/Console/Command/Compile.php(140):
        _HumbugBox5adef95163555\KevinGH\Box\Console\Command\Compile->addFiles(Object(_HumbugBox5adef95163555\KevinGH\Bo
        x\Configuration), Object(_HumbugBox5adef95163555\KevinGH\Box\Box),
        Object(_HumbugBox5adef95163555\KevinGH\Box\Console\Logger\BuildLogger),
        Object(_HumbugBox5adef95163555\Symfony\Component\Console\Style\SymfonyStyle))
#9
        phar:///bin/box.phar/src/Console/Command/Compile.php(124):
        _HumbugBox5adef95163555\KevinGH\Box\Console\Command\Compile->createPhar(Object(_HumbugBox5adef95163555\KevinGH
        Box\Configuration), Object(_HumbugBox5adef95163555\Symfony\Component\Console\Input\ArgvInput),
        Object(_HumbugBox5adef95163555\Symfony\Component\Console\Output\ConsoleOutput),
        Object(_HumbugBox5adef95163555\KevinGH\Box\Console\Logger\BuildLogger),
        Object(_HumbugBox5adef95163555\Symfony\Component\Console\Style\SymfonyStyle), false)
#10
        phar:///bin/box.phar/vendor/symfony/console/Command/Command.php(225):
        _HumbugBox5adef95163555\KevinGH\Box\Console\Command\Compile->execute(Object(_HumbugBox5adef95163555\Symfony\Com
        ponent\Console\Input\ArgvInput),
        Object(_HumbugBox5adef95163555\Symfony\Component\Console\Output\ConsoleOutput))
#11
        phar:///bin/box.phar/src/Console/Command/Build.php(34):
        _HumbugBox5adef95163555\Symfony\Component\Console\Command\Command->run(Object(_HumbugBox5adef95163555\Symfony\C
        omponent\Console\Input\ArgvInput),
        Object(_HumbugBox5adef95163555\Symfony\Component\Console\Output\ConsoleOutput))
#12
        phar:///bin/box.phar/vendor/symfony/console/Application.php(753):
        _HumbugBox5adef95163555\KevinGH\Box\Console\Command\Build->run(Object(_HumbugBox5adef95163555\Symfony\Component
        \Console\Input\ArgvInput), Object(_HumbugBox5adef95163555\Symfony\Component\Console\Output\ConsoleOutput))

[theofidry]

Thanks for reporting the issue!

Amp is optional, do box compile --no-parallel.

I didn't think there could be that kind of issue. Maybe we could either process in non-parallel when that happens but then it should be logged IMO. The files processing with PHP-Scoper is really slow, non parallelising this takes ages (easily x4 compilation time).

Alternatively, we could bail out with a more graceful message and suggest to use that --no-parallel option

Unrelated, but you should be able to simplify your config to:

compactors:
   - Herrera\Box\Compactor\Json
   - Herrera\Box\Compactor\Php
compression: GZ
output: build/puppet-enc.phar
git-commit: git_commit
git-version: git_version
git-tag: git_tag
chmod: '0755'

Removing useless files like test files & dev dependencies is sorted out by Box. It avoids any extra faff on our end, same for xdebug which is disabled by Box and the autoloader which is optimised.

what box2 has build in 63sec box3 was not able to do in over 30min

If you can dig a bit into it, share the repo or something that would be awesome. Box took 3min to process (including scoping!) 15k files and <10s without scoping when I tested it

[c33s]

with --no-parallel it works.

as discussed in the chat i think showing a warning without a bailout with an autofallback to non-parallel is a good thing if a network connection could not be established.

maybe an env would be nice to disable parallel processing globally.

a solution for the amp problem also could be, to allow to configure it more, thread count, used ports, temp file location, ...

[theofidry]

To recap my thoughts on this:

• Amp seems to be hanging out indefinitely when that happens: it's not ok especially since it's the default amp config. It should bail out at some point. I think this should be investigated and reported to Amp.
• Don't start worker when unnecessary #135 would circumvent (but not solve) this issue. The PHP and Json compactors are fast enough for the user to not care/notice the speed difference brought by the parallelisation anyway
• When Amp fails due to such an issue, either we:
  1. fallback: but there must be a logging message about it as it's a very abnormal situation
  2. bail out but gracefully and suggest the option

maybe an env would be nice to disable parallel processing globally.

I'm not against the idea, but also I would suggest it only if the previous points are not enough.

a solution for the amp problem also could be, to allow to configure it more, thread count, used ports, temp file location, ...

Like for the env: I'm not against the idea either, but I would really like that this would be the last resort to solve the issue.

[theofidry]

@c33s could you open an issue on Amp repo regarding this?

I would like to reproduce it to be able to apply a graceful message as well but I can't figure out how yet

[c33s]

the easierst way to test it for yourself is to install a strict firewall which disallows to open a port localy.

shouldn't you only have to catch Amp\Process\ProcessException?

could you open an issue on Amp repo regarding this?

should i? i am not sure what to write because i cannot really say if the problem is in how your app calls amp or if amp simply assume that all ports are open everywhere and everytime.

i think the exception handling is the part of box, because amp works per definition with sockets and ports (i assume). so box should handle if ($connection == false of catch Exception...

for the other stuff i will have to check the amp doc if it is possible or open a ticket at their repo:

• configure which ports are used
• configure the location where workers are stored
• configure the name of the workers

or maybe @kelunik (hope i picked the right maintainer for amphp) has a little time to read through this issue and shares his opinion with us.

[theofidry]

I'm not sure which exception to catch yet because it depends on how the exceptions are intercepted and re-thrown.

should i? i am not sure what to write because i cannot really say if the problem is in how your app calls amp or if amp simply assume that all ports are open everywhere and everytime.
i think the exception handling is the part of box, because amp works per definition with sockets and ports (i assume). so box should handle if ($connection == false of catch Exception...

Yes. The way Box consumes Amp can't be any simpler: wait(parallelMap($files, $processFile)). So if it doesn't thrown an exception and instead goes in an infinite loop mode it's definitely a bug there

[c33s]

oh forgot about the infinit loop xD

[kelunik]

@c33s Please open an issue at https://github.com/amphp/parallel for the infinite loop and https://github.com/amphp/process for the ports if you want.

We need to open a port for IPC communication, because Windows doesn't support non-blocking operations for pipes, which are file handles on Windows.

[theofidry]

Closing in favour of #193

[c33s]

shouldn't this be reopend, is it really a windows only? isn't it also about allowing to enable/disable multi threading?

[c33s]

.\bin\box.bat compile --no-parallel

[theofidry]

Maybe that's also the case elsewhere, I do not know but no issue has been reported so far. But since parallelism is only used when there is PhpScoper and since there is the --no-parallel option, I wouldn't say it's much of an issue