754420 - Log fetching fails in test environment

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Mark A. Hershberger (hexmode)]

Attachment: error handling fix with file_get_contents()

(Moved from Bug 752112)

I ran into trouble setting up tbpl on a virtual machine on my desktop.  I think the problem was caused by the FTP handling.

It took me a while to figure out what was going on.  Part of the problem was the hidden errors (the '@' prefix)

When I removed those, I saw the following:

  PHP Warning:  ftp_fget(): Illegal PORT command (Stingray Traffic Manager FTP
  proxy). in ..../php/inc/RawGzLogDownloader.php on line 29,

Comment 1

[Mark A. Hershberger (hexmode)]

Comment on attachment 623264 [details] [diff] [review]
error handling fix with file_get_contents()

Flagging patch for review.

Comment 2

[Mark A. Hershberger (hexmode)]

This patch would probably fix bug 740648

Comment 3

[Arpad Borsos [:Swatinem]]

Comment on attachment 623264 [details] [diff] [review]
error handling fix with file_get_contents()

Review of attachment 623264 [details] [diff] [review]:
-----------------------------------------------------------------

::: php/inc/RawGzLogDownloader.php
@@ +37,5 @@
> +    global $db;
> +    $this->assertHost();
> +    set_error_handler( 'self::customErrorHandler' );
> +    $fp = file_get_contents($this->logURL);
> +    restore_error_handler();

To be honest, this custom error handler is way too complex, the === false check below should be enough.

Comment 4

[Mark A. Hershberger (hexmode)]

(In reply to Arpad Borsos (Swatinem) from comment #3)
> To be honest, this custom error handler is way too complex, the === false
> check below should be enough.

I can understand that, but I wanted to avoid the error masking that comes from @.

Comment 5

[Mark A. Hershberger (hexmode)]

Attachment: much simpler patch

alternate patch with @ blocking the error reporting.

Comment 6

[Arpad Borsos [:Swatinem]]

Comment on attachment 623282 [details] [diff] [review]
much simpler patch

Review of attachment 623282 [details] [diff] [review]:
-----------------------------------------------------------------

What kind of error are you getting from that call? I’ve never had file_get_contents generate some runtime error for me. Might be the first one :-)

Comment 7

[Mark A. Hershberger (hexmode)]

(In reply to Arpad Borsos (Swatinem) from comment #6)
> What kind of error are you getting from that call? I’ve never had
> file_get_contents generate some runtime error for me. Might be the first one
> :-)

mah@mah-desktop:~$ grep ^error_reporting /etc/php5/apache2/php.ini
error_reporting = E_ALL & ~E_DEPRECATED
mah@mah-desktop:~$ php -v
PHP 5.3.2-1ubuntu4.15 with Suhosin-Patch (cli) (built: May  4 2012 00:38:26) 
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
mah@mah-desktop:~$ php -r 'file_get_contents("ftp://ftp.mozilla.org/none");'
PHP Warning:  file_get_contents(ftp://ftp.mozilla.org/none): failed to open stream: FTP server reports 550 Could not get file size.
 in Command line code on line 1

Comment 8

[Arpad Borsos [:Swatinem]]

Thanks! The docs are not really clear about this. :/

Comment 9

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/d3c76d604c85