ruckusing-migrations icon indicating copy to clipboard operation
ruckusing-migrations copied to clipboard

Exception handlers set no exit code

Open woru opened this issue 12 years ago • 6 comments

Exception handlers set no exit code, which causes problems when migrations are called from bash scripts.

Fix:

Index: ruckusing-migrations/lib/Ruckusing/Exception.php
===================================================================
--- ruckusing-migrations/lib/Ruckusing/Exception.php    (revision 6344)
+++ ruckusing-migrations/lib/Ruckusing/Exception.php    (working copy)
@@ -79,6 +79,7 @@
     public static function errorHandler($code, $message, $file, $line)
     {
         file_put_contents('php://stderr', "\n" . basename($file) . "({$line}) : {$message}\n\n");
+        die(1);
     }

     /**
@@ -89,6 +90,7 @@
     public static function exceptionHandler($exception)
     {
         file_put_contents('php://stderr', "\n" . basename($exception->getFile()) . "({$exception->getLine()}) : {$exception->getMessage()}\n\n");
+        die(1);
     }

 }

woru avatar Mar 20 '13 15:03 woru

Very good point. Thank you for the heads up!

On Mar 20, 2013, at 8:01 AM, woru [email protected] wrote:

Exception handlers set no exit code, which causes problems when migrations are called from bash scripts.

Fix:

Index: ruckusing-migrations/lib/Ruckusing/Exception.php

--- ruckusing-migrations/lib/Ruckusing/Exception.php (revision 6344) +++ ruckusing-migrations/lib/Ruckusing/Exception.php (working copy) @@ -79,6 +79,7 @@ public static function errorHandler($code, $message, $file, $line) { file_put_contents('php://stderr', "\n" . basename($file) . "({$line}) : {$message}\n\n");

  •    die(1);
    

    }

    /** @@ -89,6 +90,7 @@ public static function exceptionHandler($exception) { file_put_contents('php://stderr', "\n" . basename($exception->getFile()) . "({$exception->getLine()}) : {$exception->getMessage()}\n\n");

  •    die(1);
    

    }

    } — Reply to this email directly or view it on GitHub.

ruckus avatar Mar 20 '13 15:03 ruckus

Please what was this trying to fix exactly ? can we have a way to reproduce this ? This changes introduces #96 . Thanks

salimane avatar Apr 23 '13 06:04 salimane

  • env: linux
  • create a migration
  • in the up method add an invalid sql e.g. $this->execute("drop table dssdsdsdsd");
  • create a bash script or run in the command line: php ruckus.php db:migrate && echo "success"

'success' is displayed even though migrations failed

woru avatar Apr 23 '13 07:04 woru

Here is a sample code I tested with pure php

salimane at salimane-zenbook  in ~
⚛ cat a.php                 
<?php

echo a;

salimane at salimane-zenbook  in ~
⚛ php a.php                 

Notice: Use of undefined constant a - assumed 'a' in /home/salimane/a.php on line 3

Call Stack:
    0.0226     373800   1. {main}() /home/salimane/a.php:0

PHP Notice:  Use of undefined constant a - assumed 'a' in /home/salimane/a.php on line 3
PHP Stack trace:
PHP   1. {main}() /home/salimane/a.php:0

salimane at salimane-zenbook  in ~
⚛ php a.php && echo "success"

Notice: Use of undefined constant a - assumed 'a' in /home/salimane/a.php on line 3

Call Stack:
    0.0228     373800   1. {main}() /home/salimane/a.php:0

PHP Notice:  Use of undefined constant a - assumed 'a' in /home/salimane/a.php on line 3
PHP Stack trace:
PHP   1. {main}() /home/salimane/a.php:0
success

salimane at salimane-zenbook  in ~
⚛ 

Pure php is also showing that behavior.

Thanks

salimane avatar Apr 23 '13 22:04 salimane

You example is invalid. Undefined variable results in a notice not error so result code 'success' is acceptable. Try: a.php:

woru avatar Jul 03 '13 08:07 woru

please could you send a pull request.. Thanks

salimane avatar Jul 09 '13 12:07 salimane