Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Console] Add console.ERROR event and deprecate console.EXCEPTION #18140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 22, 2017
Merged

[Console] Add console.ERROR event and deprecate console.EXCEPTION #18140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions 7 UPGRADE-3.3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ Debug

* The `ContextErrorException` class is deprecated. `\ErrorException` will be used instead in 4.0.

Console
-------

* The `console.exception` event and the related `ConsoleExceptionEvent` class
have been deprecated in favor of the `console.error` event and the `ConsoleErrorEvent`
class. The deprecated event and class will be removed in 4.0.

DependencyInjection
-------------------

Expand Down
3 changes: 3 additions & 0 deletions 3 UPGRADE-4.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ Console
* Setting unknown style options is not supported anymore and throws an
exception.

* The `console.exception` event and the related `ConsoleExceptionEvent` class have
been removed in favor of the `console.error` event and the `ConsoleErrorEvent` class.

Debug
-----

Expand Down
37 changes: 31 additions & 6 deletions 37 src/Symfony/Component/Console/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use Symfony\Component\Console\Helper\Helper;
use Symfony\Component\Console\Helper\FormatterHelper;
use Symfony\Component\Console\Event\ConsoleCommandEvent;
use Symfony\Component\Console\Event\ConsoleErrorEvent;
use Symfony\Component\Console\Event\ConsoleExceptionEvent;
use Symfony\Component\Console\Event\ConsoleTerminateEvent;
use Symfony\Component\Console\Exception\CommandNotFoundException;
Expand Down Expand Up @@ -118,16 +119,40 @@ public function run(InputInterface $input = null, OutputInterface $output = null
$this->configureIO($input, $output);

try {
$e = null;
$exitCode = $this->doRun($input, $output);
} catch (\Exception $e) {
$exception = $e;
} catch (\Throwable $e) {
$exception = new FatalThrowableError($e);
}

if (null !== $e && null !== $this->dispatcher) {
$event = new ConsoleErrorEvent($this->runningCommand, $input, $output, $e, $e->getCode());
Copy link
Member

@lyrixx lyrixx Mar 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With everything on master, if you run: bin/console mldsqlkmldmksq you got this:

Fatal error: Uncaught Symfony\Component\Debug\Exception\FatalThrowableError: Type error: Argument 1 passed to Symfony\Component\Console\Event\ConsoleErrorEvent::__construct() must be an instance of Symfony\Component\Console\Command\Command, null given, called in /home/greg/dev/github.com/lyrixx/symfony/src/Symfony/Component/Console/Application.php on line 131 in /home/greg/dev/github.com/lyrixx/symfony/src/Symfony/Component/Console/Event/ConsoleErrorEvent.php:30
Stack trace:
#0 /home/greg/dev/github.com/lyrixx/symfony/src/Symfony/Component/Console/Application.php(131): Symfony\Component\Console\Event\ConsoleErrorEvent->__construct(NULL, Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput), Object(Symfony\Component\Console\Exception\CommandNotFoundException), 0)
#1 /home/greg/dev/github.com/lyrixx/symfony-standard/bin/console(28): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput))
#2 {main}
  thrown in /home/greg/dev/github.com/lyrixx/symfony/src/Symfony/Component/Console/Event/ConsoleErrorEvent.php on line 30

It's weird nobody notice that. May be I'm missing something but I really doubt, because all my repository are clean.

  1. Do you experience this behaviour too?
  2. How to patch that ? I can do a PR, but I'm not sure about the how to solve this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into this yesterday, I believe the same occurs on 2.7 with ConsoleExceptionEvent. Not sure about the better approach to fix it yet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My attempts to fix that: #22144

$this->dispatcher->dispatch(ConsoleEvents::ERROR, $event);

$e = $event->getError();

if ($event->isErrorHandled()) {
$e = null;
$exitCode = 0;
} else {
$exitCode = $e->getCode();
}

$event = new ConsoleTerminateEvent($this->runningCommand, $input, $output, $exitCode);
$this->dispatcher->dispatch(ConsoleEvents::TERMINATE, $event);
}

if (null !== $e) {
if (!$this->catchExceptions) {
throw $e;
}

if ($output instanceof ConsoleOutputInterface) {
$this->renderException($e, $output->getErrorOutput());
$this->renderException($exception, $output->getErrorOutput());
} else {
$this->renderException($e, $output);
$this->renderException($exception, $output);
}

$exitCode = $e->getCode();
Expand Down Expand Up @@ -863,17 +888,17 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
} catch (\Throwable $x) {
$e = new FatalThrowableError($x);
}

if (null !== $e) {
$event = new ConsoleExceptionEvent($command, $input, $output, $e, $e->getCode());
$event = new ConsoleExceptionEvent($command, $input, $output, $e, $e->getCode(), false);
$this->dispatcher->dispatch(ConsoleEvents::EXCEPTION, $event);

if ($e !== $event->getException()) {
@trigger_error('The "console.exception" event is deprecated since version 3.3 and will be removed in 4.0. Use the "console.error" event instead.', E_USER_DEPRECATED);

$x = $e = $event->getException();
}

$event = new ConsoleTerminateEvent($command, $input, $output, $e->getCode());
$this->dispatcher->dispatch(ConsoleEvents::TERMINATE, $event);

throw $x;
}
} else {
Expand Down
2 changes: 2 additions & 0 deletions 2 src/Symfony/Component/Console/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ CHANGELOG

* added `ExceptionListener`
* added `AddConsoleCommandPass` (originally in FrameworkBundle)
* added console.error event to catch exceptions thrown by other listeners
* deprecated console.exception event in favor of console.error

3.2.0
------
Expand Down
18 changes: 17 additions & 1 deletion 18 src/Symfony/Component/Console/ConsoleEvents.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,30 @@ final class ConsoleEvents
const TERMINATE = 'console.terminate';

/**
* The EXCEPTION event occurs when an uncaught exception appears.
* The EXCEPTION event occurs when an uncaught exception appears
* while executing Command#run().
*
* This event allows you to deal with the exception or
* to modify the thrown exception.
*
* @Event("Symfony\Component\Console\Event\ConsoleExceptionEvent")
*
* @var string
*
* @deprecated The console.exception event is deprecated since version 3.3 and will be removed in 4.0. Use the console.error event instead.
*/
const EXCEPTION = 'console.exception';

/**
* The ERROR event occurs when an uncaught exception appears or
* a throwable error.
*
* This event allows you to deal with the exception/error or
* to modify the thrown exception.
*
* @Event("Symfony\Component\Console\Event\ConsoleErrorEvent")
*
* @var string
*/
const ERROR = 'console.error';
}
112 changes: 112 additions & 0 deletions 112 src/Symfony/Component/Console/Event/ConsoleErrorEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Console\Event;

use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Exception\InvalidArgumentException;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Debug\Exception\FatalThrowableError;

/**
* Allows to handle throwables thrown while running a command.
*
* @author Wouter de Jong <wouter@wouterj.nl>
*/
class ConsoleErrorEvent extends ConsoleExceptionEvent
{
private $error;
private $handled = false;

public function __construct(Command $command, InputInterface $input, OutputInterface $output, $error, $exitCode)
{
if (!$error instanceof \Throwable && !$error instanceof \Exception) {
throw new InvalidArgumentException(sprintf('The error passed to ConsoleErrorEvent must be an instance of \Throwable or \Exception, "%s" was passed instead.', is_object($error) ? get_class($error) : gettype($error)));
}

$exception = $error;
if (!$error instanceof \Exception) {
$exception = new FatalThrowableError($error);
}
parent::__construct($command, $input, $output, $exception, $exitCode, false);

$this->error = $error;
}

/**
* Returns the thrown error/exception.
*
* @return \Throwable
*/
public function getError()
{
return $this->error;
}

/**
* Replaces the thrown error/exception.
*
* @param \Throwable $error
*/
public function setError($error)
{
if (!$error instanceof \Throwable && !$error instanceof \Exception) {
throw new InvalidArgumentException(sprintf('The error passed to ConsoleErrorEvent must be an instance of \Throwable or \Exception, "%s" was passed instead.', is_object($error) ? get_class($error) : gettype($error)));
}

$this->error = $error;
}

/**
* Marks the error/exception as handled.
*
* If it is not marked as handled, the error/exception will be displayed in
* the command output.
*/
public function markErrorAsHandled()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having this marker, can't we just set the error as null to signify that error handling is done?

Copy link
Member Author

@wouterj wouterj Feb 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it that way before, but @weaverryan suggested to use a marker:

I realize that you want a listener to ERROR to be able to "handle" the exception so that things don't continue. To do this, the listener will call $event->setException(null), which just looks like a weird way to do it.

Perhaps a $event->setExceptionAsHandled() or something to signal that the Application shouldn't continue and render the exception?

-- #18140 (comment)

{
$this->handled = true;
}

/**
* Whether the error/exception is handled by a listener or not.
*
* If it is not yet handled, the error/exception will be displayed in the
* command output.
*
* @return bool
*/
public function isErrorHandled()
{
return $this->handled;
}

/**
* @deprecated Since version 3.3, to be removed in 4.0. Use getError() instead
*/
public function getException()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use ConsoleErrorEvent::getError() instead.', __METHOD__), E_USER_DEPRECATED);

return parent::getException();
}

/**
* @deprecated Since version 3.3, to be removed in 4.0. Use setError() instead
*/
public function setException(\Exception $exception)
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use ConsoleErrorEvent::setError() instead.', __METHOD__), E_USER_DEPRECATED);

parent::setException($exception);
}
}
4 changes: 2 additions & 2 deletions 4 src/Symfony/Component/Console/Event/ConsoleEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ConsoleEvent extends Event
private $input;
private $output;

public function __construct(Command $command, InputInterface $input, OutputInterface $output)
public function __construct(Command $command = null, InputInterface $input, OutputInterface $output)
{
$this->command = $command;
$this->input = $input;
Expand All @@ -38,7 +38,7 @@ public function __construct(Command $command, InputInterface $input, OutputInter
/**
* Gets the command that is executed.
*
* @return Command A Command instance
* @return Command|null A Command instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this is a BC break.

Copy link
Member Author

@wouterj wouterj Dec 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah see #18140 (comment) :

Strictly speaking, this is a BC break. But only the public API is important for event classes, so I don't think we should consider this a BC break.

Yeah, but this only happends on the new console.error event. All other events behave the same.

*/
public function getCommand()
{
Expand Down
11 changes: 9 additions & 2 deletions 11 src/Symfony/Component/Console/Event/ConsoleExceptionEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,24 @@
* Allows to handle exception thrown in a command.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @deprecated ConsoleExceptionEvent is deprecated since version 3.3 and will be removed in 4.0. Use ConsoleErrorEvent instead.
*/
class ConsoleExceptionEvent extends ConsoleEvent
{
private $exception;
private $exitCode;
private $handled = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wouterj this prop doesn't seem to be used, should we remove it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, seems to be a left-over of the previous implementation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 2ad5923


public function __construct(Command $command, InputInterface $input, OutputInterface $output, \Exception $exception, $exitCode)
public function __construct(Command $command, InputInterface $input, OutputInterface $output, \Exception $exception, $exitCode, $deprecation = true)
{
if ($deprecation) {
@trigger_error(sprintf('The %s class is deprecated since version 3.3 and will be removed in 4.0. Use the ConsoleErrorEvent instead.', __CLASS__), E_USER_DEPRECATED);
}

parent::__construct($command, $input, $output);

$this->setException($exception);
$this->exception = $exception;
$this->exitCode = (int) $exitCode;
}

Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Console/Event/ConsoleTerminateEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ConsoleTerminateEvent extends ConsoleEvent
*/
private $exitCode;

public function __construct(Command $command, InputInterface $input, OutputInterface $output, $exitCode)
public function __construct(Command $command = null, InputInterface $input, OutputInterface $output, $exitCode)
{
parent::__construct($command, $input, $output);

Expand Down
10 changes: 5 additions & 5 deletions 10 src/Symfony/Component/Console/EventListener/ExceptionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Event\ConsoleEvent;
use Symfony\Component\Console\ConsoleEvents;
use Symfony\Component\Console\Event\ConsoleExceptionEvent;
use Symfony\Component\Console\Event\ConsoleErrorEvent;
use Symfony\Component\Console\Event\ConsoleTerminateEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

Expand All @@ -31,15 +31,15 @@ public function __construct(LoggerInterface $logger = null)
$this->logger = $logger;
}

public function onConsoleException(ConsoleExceptionEvent $event)
public function onConsoleError(ConsoleErrorEvent $event)
{
if (null === $this->logger) {
return;
}

$exception = $event->getException();
$error = $event->getError();

$this->logger->error('Exception thrown while running command "{command}". Message: "{message}"', array('exception' => $exception, 'command' => $this->getInputString($event), 'message' => $exception->getMessage()));
$this->logger->error('Error thrown while running command "{command}". Message: "{message}"', array('error' => $error, 'command' => $this->getInputString($event), 'message' => $error->getMessage()));
}

public function onConsoleTerminate(ConsoleTerminateEvent $event)
Expand All @@ -60,7 +60,7 @@ public function onConsoleTerminate(ConsoleTerminateEvent $event)
public static function getSubscribedEvents()
{
return array(
ConsoleEvents::EXCEPTION => array('onConsoleException', -128),
ConsoleEvents::ERROR => array('onConsoleError', -128),
ConsoleEvents::TERMINATE => array('onConsoleTerminate', -128),
);
}
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.