Currently the TwitterException class has a constructor function that requires PHP 5.3.

Please either:

1. Update twitter.info to include php = 5.3

2. Remove the dependency using a patch similar to:

--- a/sites/all/modules/twitter/twitter.lib.php
+++ b/sites/all/modules/twitter/twitter.lib.php
@@ -11,11 +11,13 @@ class TwitterException extends Exception {
   /**
    * Overrides constructor to log the error.
    */
-  public function __construct($message = NULL, $code = 0, Exception $previous = NULL) {
+  //public function __construct($message = NULL, $code = 0, Exception $previous = NULL) { // php 5.3
+  public function __construct($message = NULL, $code = 0) {
     watchdog('twitter', 'Unexpected error: @message', array(
       '@message' => $message,
     ), WATCHDOG_ERROR);
-    parent::__construct($message, $code, $previous);
+    //parent::__construct($message, $code, $previous); // php 5.3
+    parent::__construct($message, $code);
   }
 }
 /**
CommentFileSizeAuthor
#9 twitter-1814642-fatals_in_exception_handling.patch1.28 KBxurizaemon
#10 twitter-1814642-fatals_in_exception_handling.patch1.28 KBxurizaemon

Comments

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
Status: Active » Needs work

I'm OK with requiring 5.3, we need JSON_BIGINT_AS_STRING for #985544: {twitter}.twitter_id incompletely stored (final digits are zeroes) due to json_decode limitation in PHP<5.3.

EDIT: Whatever, now php.net says JSON_BIGINT_AS_STRING is PHP5.4 :P

fizk’s picture

fizk commented

I suppose requiring 5.4 wouldn't work for most users. If we're rolling back the use of JSON_BIGINT_AS_STRING, I'd suggest also rolling back the requirement of 5.3 by changing __construct in twitter.lib.php

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented

I wouldn't be surprised if that's a docs bug on php.net, I recall the version for that feature was vaguely documented during 5.3 development.

Losing JSON_BIGINT_AS_STRING would be a pain so I'm not suggesting we take that route unless we get report of a common configuration where it's omitted.

fizk’s picture

fizk commented

I think the docs are correct. JSON_BIGINT_AS_STRING was committed on May 21, 2010:

https://github.com/php/php-src/commit/6eb4218433e5b2af2974648b31ca434b1a...

It shows up in the 5.4 branch:

https://github.com/php/php-src/blob/PHP-5.4/ext/json/JSON_parser.c

but not in the 5.3 branch:

https://github.com/php/php-src/blob/PHP-5.3/ext/json/JSON_parser.c

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented

Apologies - you're right. Having tested, none of the stock PHP5.3 I have to hand have JSON_BIGINT_AS_STRING.

We committed a different partial fix for that issue, so it isn't relevant to this one.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented

@fizk (or anyone), can you confirm the fix above for 5.2? It looks trivial but I'd like to know it works. Requiring 5.3 is convenient for us maintainers, but with a large install base the right thing to do is probably not bump requirements over a couple lines of code.

fizk’s picture

fizk commented

@grobot, this fix?

--- a/sites/all/modules/twitter/twitter.lib.php
+++ b/sites/all/modules/twitter/twitter.lib.php
@@ -11,11 +11,13 @@ class TwitterException extends Exception {
   /**
    * Overrides constructor to log the error.
    */
-  public function __construct($message = NULL, $code = 0, Exception $previous = NULL) {
+  //public function __construct($message = NULL, $code = 0, Exception $previous = NULL) { // php 5.3
+  public function __construct($message = NULL, $code = 0) {
     watchdog('twitter', 'Unexpected error: @message', array(
       '@message' => $message,
     ), WATCHDOG_ERROR);
-    parent::__construct($message, $code, $previous);
+    //parent::__construct($message, $code, $previous); // php 5.3
+    parent::__construct($message, $code);
   }
 }
 /**
xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented

Yes. Or (if we see an advantage in passing $previous back)

-    parent::__construct($message, $code, $previous);
+    if (!is_null($previous)) {
+      parent::__construct($message, $code, $previous); // php 5.3
+    }
+    else {
+      parent::__construct($message, $code);
+    }

OAuth 6.x-3.x requires 5.2. Looks like OAuth 7.x-4.x will make more use of 5.3: #1591692: Replace current OAuth library.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new twitter-1814642-fatals_in_exception_handling.patch1.28 KB

Patch for improved exception handling, tested on 5.2.

A related issue I found, which could be split off to another issue if need be. HTTP Unauthorized would throw "Cannot use string offset as an array" on line 215 as a result of 211ef363. This is because $data == ' ' from Twitter::parse_response() (json_decode returns ' ' if submitted ' ').

Perhaps we should make parse_response() only return if it obtains a meaningful result.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
Title: Document requirement of PHP 5.3 or remove dependency » Invalid handling of exceptions causes fatal errors on PHP5.2
Status: Needs work » Needs review
StatusFileSize
new twitter-1814642-fatals_in_exception_handling.patch1.28 KB

replaces is_string($data) with !is_array($data)

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented

Should be resolved with commits f71e651 and ca98fca from last night, 71e5664 in 6.x-3.x.

Marking fixed as I believe this is resolved, have directed @bwoods here from his report that the issue persists on 5.2.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
Status: Needs review » Fixed

~

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.