The problem: Anonymous users can't add things to cart (on first try, anyway)

After dealing with this issue for some time now, and experiencing it on our site, I believe I've found the root issue. For a complete method of how to reproduce the issue (somewhat fixed on our site since I am implementing the fixes I come across as I get them):

http://www.ubercart.org/forum/bug_reports/8693/anonymous_user_add_cart_d...

When an Add to Cart button is pressed, the submit handler calls uc_cart_get_id() which retrieves the $_SESSION['uc_cart_id'] for the current user. Unfortunately, when a user is Anonymous, Drupal doesn't seem to allow the cookie to be set, even if you have Normal core caching disabled. It seems that this cookie (containing the $_SESSION['uc_cart_id'] variable) needs to be set before the add_to_cart_submit handler is called by an Add to Cart button. (Oddly enough, Buy it Now buttons and Cart Links are not affected here, at least, this was the case in my testing.)

The solution: Implement hook_boot()

The solution is really simply: I added a function uc_cart_boot(), which is run even on cached pages, that simply calls uc_cart_get_id(). Since this gets called on every page load (even on cached pages) the cookie is retrieved if it exists, or set if it does not. I didn't notice any additional overhead, and it gives us the benefit of allowing all anonymous users to start with a cookie containing an empty cart that uc_cart.module can now act upon.

/**
 * Implementation of hook_boot().
 */
function uc_cart_boot() {
  if (!isset($_SESSION['uc_cart_id'])) {
    uc_cart_get_id();
  }
}

Issues with Boost static page caching enabled

Caveat: Boost.module still has trouble, because it serves - via .htaccess rules - static .html pages from Apache, and so hook_boot() is never invoked. See my comment #7 here: #586210: Set session cookie

... which is essentially the issue that needs to be resolved in order to get Anonymous Add to Cart buttons working with Boost. My suggestion is to include some client-side JS that calls a non-cached AJAX callback. This AJAX callback would run the uc_cart_get_id() function, thereby allowing the anonymous user to set a cookie despite being on a static page. (It's still a theory, I have yet to test it out. Will post back here once I've done so.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

torgosPizza’s picture

AJAX functionality

Another thing I've just discovered; I created a function that is associated with an ajax menu callback:

function uc_cart_get_id_ajax() {
  $cid = uc_cart_get_id();
  return drupal_json($cid);
}

.. so, for an anonymous user, we'd want to hit this callback (via the current testing URL of /cart/ajax/id) which would fire the uc_cart_get_id() function, thereby setting the cookie. Right? Wrong. Here's why:

/**
 * Return the unique cart_id of the user.
 */
function uc_cart_get_id() {
  global $user;

  if ($user->uid) {    
      return $user->uid;    
  }
  elseif (!isset($_SESSION['uc_cart_id'])) {
    $_SESSION['uc_cart_id'] = md5(uniqid(rand(), TRUE));
  }

  return $_SESSION['uc_cart_id'];
}

All this function does is return the uid of the current user if they are logged in. The problem is that it still does not set the session cookie if it's been deleted (or never existed, in the rare event that hook_boot() got missed). The solution is easy, add a condition to look for the session variable, and set it if it doesn't exist:

/**
 * Return the unique cart_id of the user.
 */
function uc_cart_get_id() {
  global $user;

  if ($user->uid) {
    if (!isset($_SESSION['uc_cart_id'])) {
      $_SESSION['uc_cart_id'] = $user->uid;
      return $user->uid;
    }
  }
  elseif (!isset($_SESSION['uc_cart_id'])) {
    $_SESSION['uc_cart_id'] = md5(uniqid(rand(), TRUE));
  }

  return $_SESSION['uc_cart_id'];
}

Is there a reason why UC shouldn't be doing this? I think it never hurts to check, and by doing so, will open us up to more AJAX functions that won't need to set the cookie on their own.

(See my comment further down: #679422-5: hook_exit() causes Ubercart "Add to Cart" to fail)

torgosPizza’s picture

Getting Anonymous add-to-cart working with Boost

Well, I got it to work. Here's how:

In uc_cart.module

  // Add to uc_cart_menu()
  $items['cart/ajax/id'] = array(
    'title' => 'Add cart ID',
    'description' => 'Add a cart id session cookie',
    'page callback' => 'uc_cart_get_id_ajax',
    'access arguments' => array('access content'),
    'type' => MENU_CALLBACK,  
  );

// Add anywhere (Ajax callback functioin)
function uc_cart_get_id_ajax() {
  $cid = uc_cart_get_id();
  return drupal_json($cid);
}

// Slight modification of uc_cart_get_id():
function uc_cart_get_id() {
  global $user;

  if ($user->uid) {
    if (!isset($_SESSION['uc_cart_id'])) {
      $_SESSION['uc_cart_id'] = $user->uid;
      return $user->uid;
    }
  }
  elseif (!isset($_SESSION['uc_cart_id'])) {
    $_SESSION['uc_cart_id'] = md5(uniqid(rand(), TRUE));
  }

  return $_SESSION['uc_cart_id'];
}

// In uc_cart_init()
drupal_add_js(drupal_get_path('module', 'uc_cart'). '/uc_cart.ajax.js');

New file, a single line of jQuery that fires the callback (could even be rolled into uc_cart.js):

$Id; File: /sites/all/modules/ubercart/uc_cart/uc_cart.ajax.js
$(document).ready(function() { $.get("/cart/ajax/id?nocache=1"); });

Clear your menu cache, and then visit any page with Boost and Caching enabled. (Make sure you have turned OFF "cache pages with variables in the URL" - haven't tested it yet). What happens is:
1) hook_init() always aggregates JS and CSS, no matter the cache settings.
2) Even with Boost enabled, that hook is run, adding the JS to the static html.
3) Because the JS includes a $.get request to a non-cached page that simply runs the callback...
4) ... the uc_cart_get_id() function is fired, setting the cookie.

Since the JS is handled by the browser, not the server, and the ajax callback is not being cached by Boost, you can be fairly certain that the cookie is being set and is unique for each anonymous visitor.

Feedback and further testing is welcome. I'm running it on our live site to see how it works, even running multiple browsers to test as different Anonymous users shows positive results: no cart crossover, no Add to Cart failures at all.

mikeytown2’s picture

Title: Anonymous users "Add to Cart" does not work - cookie not being set (fix included) » Anonymous users "Add to Cart" does not work - session var not being set (fix included)

subscribe

torgosPizza’s picture

Here's a patch containing all of my changes.

Also attached is the JS file that will need to be included within the sites/all/modules/ubercart/uc_cart directory (rename it to uc_cart.ajax.js).

torgosPizza’s picture

My modification of uc_cart_get_id() needs to be changed. The way it's implemented as I've done, it will not transfer an anonymous cart to a user account if a user logs in with items in their cart. (Not good.)

So, here's how the code should look, a good compromise I think:

/**
 * Return the unique cart_id of the user.
 */
function uc_cart_get_id() {
  global $user;

  if ($user->uid) {
    $_SESSION['uc_cart_id'] = $user->uid;
    return $user->uid;
  }
  elseif (!isset($_SESSION['uc_cart_id'])) {
    $_SESSION['uc_cart_id'] = md5(uniqid(rand(), TRUE));
  }

  return $_SESSION['uc_cart_id'];
}
torgosPizza’s picture

FileSize
2.04 KB

Updated patch. USE THIS ONE and the JS file from #4.

hunthunthunt’s picture

@torgosPizza

Great work on this - was having the exact same issue.

Can you please confirm, to implement this patch I need:

1. The patch form #6 plus
2. The JS from #4

Thanks,

torgosPizza’s picture

Yup, that's all :)

dmower’s picture

I'm having this same issue, but only on my live server and not the development one for some odd reason. This patch doesn't seem to solve the problem for me, anything else that has to be changed (settings?) besides just patching the files?

torgosPizza’s picture

What kind of caching do you use? And no, you shouldn't have to do anything else. Do you have a site we can take a look at?

EDIT to add: Interestingly enough, I can't reproduce this with core cache on my local install of Drupal (on a MAMP stack). Very strange, since I was able to reproduce it countless times on our live site. I wonder if this issue is related? Perhaps it's an OS-specific issue? I'm looking for some more details to see if I can reproduce it on a plain, default install.

AFAIK none of our modules would have this type of effect on session variables being set... I'll see if I can nail down something conclusive.

dmower’s picture

Currently I'm not using any cache on either the dev server or the live server, our site has been changing so much lately that i found it counter productive to enable it just yet.

The dev server is running Debian while the live server is Red Hat Enterprise, but the actual services are the same, php5, mysql and apache2, config files for apache all seem to be the same for those particular sites, and general config is also the same.

I also noticed (using firebug/webdeveloper toolbar) that the cookies ARE being set for the anonymous users, ubercart just seems to be ignoring them, or not finding what it needs in them.

Unfortunately I don't have a public site to share, the site itself is public but the cart itself doesn't have any public facing navigation as it's for internal use only.

dmower’s picture

Seems that turning on 'normal' cache in drupal makes it function as expected, however it's still odd that with caching off, it works fine on the development server.

torgosPizza’s picture

IIRC, I did see a few times when the issue would occur whether core cache was turned on or off, that didn't seem to make a huge difference, because as you said - Ubercart seems to ignore the session variable being set, at least until you submit an Add to Cart form for the first time.

You're saying though, that with your caching (core) turned off, you still have the issue? Add to Cart doesn't work for Anonymous users?

I would love to have another site to test against, but if it's a production site you'd want to be careful. Mainly I just added dsm()'s to the uc_cart.module to see what the status of $_SESSION was throughout the process of viewing pages, adding to cart, etc.

dmower’s picture

Ok, so now the problem has come back and now anonymous users can't add anything to a cart regardless of cache settings on the live server. Only thing I changed was enabling the cart_links module and adding products.

www.descentpass.com/shop and /cart to see for yourself, the links aren't public anywhere on the page but going directly to them works.

ETA: also, I'd be willing to test some if I knew what to do, and where. Live server or not, I can't continue unless I get this working :)

torgosPizza’s picture

Do you have any settings that are unique in your settings.php file? Like for your base url, or the cookie domain? Anything like that?

Basically the issue is in uc_cart.module, I think. Do something like this in your hook_menu in uc_cart (uc_cart_menu function).

drupal_set_message(print_r($_SESSION, true));

Basically what I did to test was put that in the hook_menu so that on every page you can see what it looks like. There should be a value in there - "uc_cart_id" - that exists when you add something to your cart.

If you aren't comfortable testing I'd be more than happy to take a look.

dmower’s picture

Ok, putting that line in the uc_cart_menu function didn't seem to do anything at all, putting it in uc_cart_view() (uc_cart.pages.inc) however produces a message on each page.

On the test server I get

Array ( [uc_cart_id] => 89e776eca7a2356e096a3f442a198758 )

There are no products in your shopping cart.

As well as additional entries when I add something to the cart.

On the live server I do get a uc_cart_id, every time i hit the /cart page, but nothing else

dmower’s picture

I just noticed that if I simply stay on the /cart page and hit refresh a few times, the session id changes every time. Isn't that supposed to stay the same until the session expires?

Doing the same thing on the test server return the SAME session id every time, so it seems for some reason the live server is dropping the sessions

torgosPizza’s picture

Are you on Drupal 5 or Drupal 6?

I'm not sure why having it in hook_menu wouldn't do anything - that hook should get called on every page view.

EDIT: Nevermind, I see that you're on D6. Are you sure you're not using any other caching modules? hook_boot and hook_menu should at least display a message. Make sure you're using a drupal_set_message and just not printing it as output.

And yes, the uc_cart_id should get set once and that's it. I get this feeling that you have some other issue with cookies and sessions, whether the issue be server-related or settings.php related. Are you running the actual site in a subfolder or anything? (Like a shared host site?)

dmower’s picture

Yes, D6. No caching modules. The base url and cookie domain are set in the settings.php, and it's a dedicated server with multiple sites/domains on it (dev server is setup the same way).

At one point I did try commenting out the cookie domain just to see, and it didn't seem to have any affect.

$base_url = 'http://www.descentpass.com';

$cookie_domain = 'www.descentpass.com';

Perhaps I put the code in the wrong spot?

function uc_cart_menu() {
  drupal_set_message(print_r($_SESSION, true));
  $items = array();

*snip*
torgosPizza’s picture

Ah, well in that case, maybe hook_menu isn't the best place. (Come to think of it, I don't think hook_menu gets called except on module enable/disable or when menu_router is updated.) Look for a hook_init and put the code there. But, then again, if simply being on /cart causes the uc_cart_id to keep changing, I can think of two things: either the session isn't being stored in PHP, or there's an issue with your database table for uc_cart.

Do you get any errors in Watchdog? Can you check your Apache error logs also? To me it seems like either the session is getting destroyed for some reason, or the database table uc_cart_products isn't having rows inserted or updated. Maybe it's an issue with the session table as well?

Also, another issue is with the uid of anonymous user 0. Sometimes, the users table (if you are importing it from a dev site) will update that to some other value. Check out the thread in my original post for what that looks like. But basically you want to make sure your first row in {users} is uid = 0.

dmower’s picture

Oddly enough, uid 0 was missing from the live server. I've never transfered user data between the sites, so that's a little strange.

Either way, that seems to have fixed it (hopefully for good this time)

Thanks for all the help on this :)

torgosPizza’s picture

Yeah it does it because of the schema.

Anyway, now that you have the userid = 0 in your users table, let me know if you get the same issue this original post was meant for :)

tintstb’s picture

What patch should I use to fix the Boost problem? Does #5 applyes to #2 ?

torgosPizza’s picture

Use the patch from #6 and the .js file from #4.

p4trizio’s picture

Hi, I'm trying to get it to work but without success.
The website: www.musikzoneshop.com
If anyone want to give it a try... thanks!

torgosPizza’s picture

I don't believe your issue is related.

Things are still added to your cart (your AJAX cart block is working fine!) but when you go to http://www.musikzoneshop.com/shop/cart the cart form doesn't display at all. However when you proceed to checkout, the items are of course there (as the block says).

This issue is something completely different. Please read my original post. You should at least see a "There are no items in your cart" message, but you don't even see that, which makes me think you have a theme function that is overriding your cart display. It might be related to the Ajax cart block, but I'm not sure. Try disabling that module and see if the cart page comes back.

This is not the same as "adding an item to the cart doesn't actually add anything" which is what this issue is in regard to.

p4trizio’s picture

Hi, thanks for the fast reply!!!
The cart is at www.musikzoneshop.com/cart

I'm sorry if it is not related... I actually didnt notice! I was searching for a solution to display the ajax_cart with boost module in the proper way.
I have no overriding functions on my theme or other modules enabled... at least that I know.

Right now my problem is that the cart block is cached in each page in different ways. Isn't this the right post to discuss about it?
Thank you

P

torgosPizza’s picture

No, the cart block is a different issue. I believe it's discussed here: #482986: Adding products for anonymous users not working and there are many discussions about it on the UC forum:
http://www.ubercart.org/search/apachesolr_search/anonymous+cart+block

You may also consider this: http://www.ubercart.org/contrib/13521

But yeah, this issue only deals with "items are not being added to the cart", not any "block display" issue. Somewhat different. :)

I get the feeling you're using uc_ajax_cart. In that case you should post your issue for that module: http://drupal.org/project/uc_ajax_cart

p4trizio’s picture

Thanks for your patience and all the tips!

babbage’s picture

I haven't read all the posts here, but just to say that in my case it was 100% Boost that was the problem; turning off Boost caching solved the issues immediately. Turning it on and off and testing confirmed that Boost was the culprit. Interested in any solution as I'd like to use Boost but at least we are back on track for launch!

torgosPizza’s picture

That's the point of this thread. Boost is required on our site, which is what the patches in this thread are about. (Hence, reading the posts contained here would be in your best interest.) There are two things you'll need to help test, a patch and a JS file. Read above for more details.

gooddesignusa’s picture

subscribe

jeremy.zerr’s picture

FileSize
95 bytes

I have been experiencing this same problem, here is what I expeirenced on UC 2.2. I found that I also had a bad uid 0 in my users table, the anonymous user had been given the uid of 40, likely due to export/import of db during development.

1) With bad uid 0 and no #6/#4 fix:
Caching enabled: anonymous cart says "your shopping cart", and couldn't add items to cart
No caching enabled: anonymous cart says # and cost of items, but couldn't add items to cart

2) Fixed uid 0 and no #6/#4 fix
Caching enabled: anonymous cart says "your shopping cart", but can add items to cart and see by visiting the "cart" URL directly
No caching enabled: anonymous cart says # and cost of items, and can add items to cart, can see them on cart page, and it updates the cart # and cost

3) Left uid 0 broken and applied #6/#4 fix
Same results as 1)

4) Fixed uid 0 and applied #6/#4 fix
Same results as 2)

So I really have no idea what the combination of #6/#4 is supposed to help because in my case, it didn't do a thing. Really the root of the problem was the uid 0 being wrong.

I checked on quite a few of my sites, and all sites that I did a export/import db to create the site had uid 0 messed up. Only a site that I installed through the Drupal installer had uid 0 correct. So really, I think this is the root cause of the issue for me. However, the situation still is not ideal in my mind because in state 2), with caching enabled, the cart still says "your shopping cart". By the way, the uid fix was easy, my anonymous uid had been changed to 40 (just look at the first row of the users table), so I did UPDATE users SET uid = 0 WHERE uid = 40.

Also, I made sure I was investigating the returns of the AJAX call inserted by the #6/#4 fix, and it returned the UID for a logged in person and a session ID for an anonymous person.

On a side note, the code in the javascript file from #4 will not work in all cases because it assumes the cart is at "/cart", so if you have a Drupal site with a root of http://www.example.com/drupal, then it looks at http://www.example.com/cart when it instead needs to look at http://www.example.com/drupal/cart. This problem can be worked around by using the Drupal global javascript var Drupal.settings.basePath. See attached uc_cart.ajax.js replacement.

Jeremy Zerr - http://www.zerrtech.com

torgosPizza’s picture

Jeremy,

Thanks for the post. Are you also using Boost, or not?

jeremy.zerr’s picture

I do not have Boost as a module on my site.

torgosPizza’s picture

Thought I'd ask :)

TR’s picture

Minor changes to the jQuery in #33:

1) We don't have to wait for the DOM to load, so I removed the $(document).ready().

2) I changed the $.get() to $.post() so the Ajax request wouldn't be subject to caching on the client side. And it's more proper semantically as well, since you're using it to initiate a session on the server. I don't know where you came up with the ?nocache=1 suffix, but since that suffix is static you'll always be requesting the same URL, and it won't have the effect you desire. You could have used ? followed by a random number or a unique string (a timestamp, for instance), but "nocache" isn't a key word or anything ...

3) As per Drupal recommendations (http://drupal.org/update/modules/6/7#javascript_compatibility), I put the definition of $ into local scope so the jQuery code doesn't conflict with other JavaScript libraries.

I think this should go into uc_cart.js - no sense loading yet another script separately from the page. I'm just going to inline my version of the script here, because this issue still needs a proper patch to address all the points:

(function($){ $.post(Drupal.settings.basePath + 'cart/ajax/id'); })(jQuery);
mikeytown2’s picture

nocache is for boost; if it sees that it will not cache it. If using a post it won't cache it so safe to remove in this case.

nainil’s picture

Which file would it be where you would add this code?

/**
* Implementation of hook_boot().
*/
function uc_cart_boot() {
if (!isset($_SESSION['uc_cart_id'])) {
uc_cart_get_id();
}
}

Thanks in advance.

babbage’s picture

Should that be hook_boost by any chance? ;)

TR’s picture

mikeytown2’s picture

uc_cart.* file
http://api.drupal.org/api/function/hook_boot/6

I'm always open to new additions to the boost module; it's extremely rare for me to fully turn down a boost patch (I don't think I ever have).

torgosPizza’s picture

Yeah and it looks like this issue isn't just for Boost; it may be just an issue with Ubercart not setting the session var / cookie until after an UC page has been visited once. Generally:

1) Clear your cookies.
2) Visit any page with a product "Add to Cart" button on it.
3) Add the product. Cart will be empty.

I've found that generally doesn't work unless you use a cart link. Someone I know is having this same exact issue with core cache disabled and he's not using Boost, AFAIK, so it seems to be a core UC issue that's worth resolving.

alexis’s picture

Any ideas on how to make this work for Ubercart on Drupal 5?

alexis’s picture

Ok, it seems this is working for me on Drupal:

1. I added this code for the 'else' of uc_cart_menu because there's no hook_boot on Drupal 5:

  else {
    if (!isset($_SESSION['uc_cart_id'])) {
      $cart_id = uc_cart_get_id();
    }
  }

2. I noticed Ubercart for Drupal 5 has a slighly different uc_cart_get_id and there wasn't a $_SESSION['uc_cart_id'] set so I replaced uc_cart_get_id which was like this on Ubercart for Drupal 5:

function uc_cart_get_id() {
  global $user;
  if ($user->uid) {
    return $user->uid;
  }
  elseif ($sid = session_id()) {
    return $sid;
  }
  // What to do if neither of these work? -RS
}

with this, that is the version from Ubercart for Drupal 6:

function uc_cart_get_id() {
  global $user;

  if ($user->uid) {
    return $user->uid;
  }
  elseif (!isset($_SESSION['uc_cart_id'])) {
    $_SESSION['uc_cart_id'] = md5(uniqid(rand(), TRUE));
  }

  return $_SESSION['uc_cart_id'];
}

I've tested and so far the 'no products in shopping cart' problem seem to be gone.

Thanks torgosPizza for your great research into this problem!

jazzitup’s picture

After I've found this page, applied the patch without any effect, I've found out that this could be my issue: #391534: Fails to add to cart when using views global:random display Maybe this info could help someone as well.

tristan_a1’s picture

I am also having this problem with anonymous users getting "There are no products in your shopping cart.".

Where do i find the file i need to edit the user id and change it to 0?

Can i just copy the "uc_cart.anon_.patch" and "uc_cart.ajax_.js" files to this directory sites/all/modules/ubercart/uc_cart ??

Thanks

Tristan

torgosPizza’s picture

Version: 6.x-2.2 » 6.x-2.x-dev

I am researching this issue again because we need to get anonymous checkout working.

I tested the patch at #377798-67: Cart block always starts a session and that does not help if I'm not using a Cart Block. However, adding a new implementation of hook_boot() into uc_cart.module sets the cookie and shows a correct cart with all caching (core and Boost) disabled:

function uc_cart_boot() {
  if (!isset($_SESSION['uc_cart_id'])) {
    uc_cart_get_id();
  }
}

If I add this function in, and follow the same steps (1. Clear cookies. 2. Visit a product page with an "add to cart" button. 3. add product) the product does get added correctly. Have not tested with Boost yet.

This should fix the core issue, however Boost will need to be patched to look for the Ubercart Cart module and if so, ping a new menu item that will set the session var.

Island Usurper’s picture

I've found the anonymous cart to work just fine without Boost. As far as I can see, the cart id is generated when it needs to be, when something is added to the cart.

I don't see a problem with making an AJAX callback to generate a session id for Boost, I guess, but I don't see a need for the hook_boot(). I also don't know about setting the session cart id in uc_cart_user_login_form_submit(). Once the user is logged in, I'm not sure that variable is even used anymore. But it does make a good sanity check regardless.

I guess I feel like I need more information, because it doesn't seem like a problem in general. If there's another situation besides using Boost that causes the cart to not work, then we can do something to fix it.

torgosPizza’s picture

Right, well that's the thing - the tests I've done involve Boost not being part of the equation, necessarily. (It was when I started the issue, though.) The post I made above I did with core caching off and Boost caching off, as well. For the life of me, I can't see what else that would be causing it.. but what I do know is that, from the research and testing I did a while back, the sessions were not being created for me until after I visited the cart for the first time.

EDIT: You're right, on a fresh install, with nothing enabled except CCK and Ubercart, this issue does not occur. Mea culpa. Will go back to digging.

I AM still able to reproduce this on our live development server, when I remove the hook_boot from uc_cart.module (the hook_boot I created). I'm disabling Boost, etc. again to help further narrow this down. But of course the dev server has other modules enabled besides just the required ones.

Could other commenters in this thread let me know if they're still seeing this issue, using the latest Dev of Ubercart? Let me know if you're using (or not) Boost or any other caching mechanisms like memcache, etc. (That shouldn't make a difference, per se, but it'll help to rule out the obvious.)

torgosPizza’s picture

After finding this similar issue at #513860: Wrong redirect after user registration., I think I've found the issue. In Boost, there is a hook_exit() being performed:

function boost_exit($destination = NULL) {
  // Check that hook_exit() was invoked by drupal_goto() for a POST request:
  if (!empty($destination) && $_SERVER['REQUEST_METHOD'] == 'POST') {

    // Check that we're dealing with an anonymous visitor. and that some
    // session messages have actually been set during this page request:
    global $user;
    if (empty($user->uid) && ($messages = drupal_set_message())) {
      // FIXME: call any remaining exit hooks since we're about to terminate?

      $query_parts = parse_url($destination);
      // Add a nocache parameter to query. Such pages will never be cached
      $query_parts['query'] .= (empty($query_parts['query']) ? '' : '&') . 'nocache=1';

      // Rebuild the URL with the new query string.  Do not use url() since
      // destination has presumably already been run through url().
      $destination = boost_glue_url($query_parts);

      // Do what drupal_goto() would do if we were to return to it:
      exit(header('Location: ' . $destination));
    }
  }
  // Set watchdog error if headers already sent
  if (BOOST_ASYNCHRONOUS_OUTPUT && $GLOBALS['_boost_cache_this'] && headers_sent($filename, $linenum) && !boost_headers_contain('Location: ') && BOOST_VERBOSE >= 7) {
    watchdog('boost', 'boost_exit() Debug: Headers already sent in @filename on line @linenum. Asynchronous Operation will not be used.', array('@filename' => $filename, '@linenum' => $linenum));
  }
}

If I make the exit() call just:

  header('Location: '.$destination);

... this WORKS! My products are added to the cart and everything seems to go smoothly. My guess is that other users who are having this similar issue, have a contrib that is installed which invokes hook_exit(), which also calls an exit() (the php function). exit(), according to php.net, terminates the script. This appears to break Ubercart's add to cart functionality, I'm guessing because there are so many other scripts involved in the process, and perhaps the order of the scripts' execution is throwing things up.

I'm going to make this an issue for Boost, for now. If other users experience the same issue, my guess is that it's more of an issue for the contrib calling hook_exit() than it is for Ubercart itself.

torgosPizza’s picture

Title: Anonymous users "Add to Cart" does not work - session var not being set (fix included) » hook_exit() causes Ubercart "Add to Cart" to fail
Project: Ubercart » Boost
Version: 6.x-2.x-dev » 6.x-1.x-dev

miketown2,

I'm not sure how you'd like to handle this. I'm not sure what the ramifications are/would be of removing exit() from that function, if any. Please chime in with your thoughts on this latest development.

mikeytown2’s picture

hmm so aborting the rest of the hook_exit calls causes bad things to happen... This is in here as a hack for drupal_goto for the most part. Is there a hook_exit in ubercart?

mikeytown2’s picture

looking at drupal_goto() it gets its $url from url(). This can be modded by custom_url_rewrite_outbound() or language_url_rewrite(). I could try one and then the other before reverting to the fallback of the exit call.

@torgosPizza
Try it when ?destination= is set. This is what the hook_exit code is for.

EDIT: Well technically its here to add in nocache=1 so the next page after a POST is not cached. Since I have the detection of drupal_set_message this code is not as critical as it used to be... I think there could be a better way to do this.

torgosPizza’s picture

Indeed, there are two:

// In uc_store.module
/**
 * Implementation of hook_exit().
 */
function uc_store_exit() {
  // Save the current request for tracking paths on subsequent page requests.
  // When HTTP_REFERER is set, the session version is not; and vice versa.
  if (referer_uri() == '') {
    $protocol = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off') ? 'https' : 'http';
    $_SESSION['uc_referer_uri'] = $protocol .'://'. $_SERVER['SERVER_NAME'] . $GLOBALS['base_path'] . $_GET['q'];
  }
  else {
    if (isset($_SESSION['uc_referer_uri'])) {
      unset($_SESSION['uc_referer_uri']);
    }
  }

  // Save the timestamp of the last access.
  // $_SESSION['uc_last_access'] = time();
}


// In uc_cart.module
/**
 * Implementation of hook_exit().
 *
 * Code from CacheExclude - http://drupal.org/project/cacheexclude
 */
function uc_cart_exit() {
  global $base_root;

  $pages = array('cart', 'cart/checkout', 'cart/checkout/review', 'cart/checkout/complete');
  $this_page = request_uri();
  foreach ($pages as $page) {
    if ($page && strstr($this_page, $page) !== FALSE) {
      cache_clear_all($base_root . $this_page, 'cache_page');
      return;
    }
  }
}

Neither of which appear to actually exit() from anything, they seem to prevent caching of the cart pages (in uc_cart) and tracking referring paths in the case of uc_store.

torgosPizza’s picture

@torgosPizza
Try it when ?destination= is set. This is what the hook_exit code is for.

Nope, no change there. I went to a product page with ?destination=cart at the end, but the "add to cart" process was still aborted.

EDIT: I should mention that whatever I change "destination" to, the drupal_goto does fire and I am taken to the correct destination.. it just appears that exit() is aborting the actual form's processing.

mikeytown2’s picture

what about a drupal_set_message call? does uid=0 get the message then?

Can you also try calling this code in boost_exit

    if (empty($user->uid) && ($messages = drupal_set_message())) {
      // FIXME: call any remaining exit hooks since we're about to terminate?
  uc_store_exit();
  uc_cart_exit();

      $query_parts = parse_url($destination);
torgosPizza’s picture

The user does get the message, but here's the rub:

Earlier in my testing, a drupal_set_message() would only help if it were executed higher than Boost Ubercart. (I think.) For example if I set a drupal_set_message() in uc_cart (like in the new uc_cart_boot() function I wrote, or in any other UC function) it does nothing. But when I put the message into includes/form.inc to show me the interior of a $form object, it would work (and a product would get added to the cart), because Boost was seeing those a drupal messages. Anything else, it doesn't seem to behave. I mean, the message itself is displayed (as it should) but the rest still breaks.

Adding those two calls to hook_exit() also had no effect.

mikeytown2’s picture

Status: Active » Needs review
FileSize
2.59 KB

sounds like I should make this an option for now till I figure out the best way to fix it.

mikeytown2’s picture

FileSize
2.59 KB

This has the correct default value

torgosPizza’s picture

Cool, thanks. That looks like a good workaround for now. It makes me wonder if there is a way this could be handled in Ubercart that is better, perhaps if the API was more well-defined? I'm not 100% on the process, so it's hard to see where the breakdown actually occurs.

Thanks for taking a look at this.

mikeytown2’s picture

mind testing it so I can commit this?

torgosPizza’s picture

Tested it against 6.x-1.x-dev and it patched fine. Worked like a charm.

Once I get more time I'll try to hack at this a little more deeply, to see if there's a better way to go about it - both within Ubercart and Boost.

Many thanks!

mikeytown2’s picture

Status: Needs review » Fixed
torgosPizza’s picture

Cool.

And hey, FYI - after talking with tr and IslandUsurper about this issue in the Ubercart IRC, I guess they were wondering why exit() was being called at all within hook_exit()? Since exit seems to shutdown before buffers are flushed, that would explain how things are breaking. Any thoughts? Is exit() really necessary? (I'm assuming you've found cases where it was needed... offhand I don't know what that would be though. Your insight is appreciated!)

mikeytown2’s picture

exit is code that was inherited when I took over boost. The reason why its needed is logical. If you do a form submit and the landing page is cached then you want to add in a nocache=1 at the end so you skip the boost cache and get any kind of message back.

Example:
Submit something to "/form-process" via POST. form-process does its thing and does a "drupal_goto()" with a "drupal_set_message('thanks')". Without the altered location header then you get the cached front page because drupal_goto sets the location header again.

header('Location: '. $url, TRUE, $http_response_code);

since the second parameter is replace the old header that we just set would be overwritten by this new one at the end of drupal_goto. http://php.net/header
Because of this, there is an exit call so the redirect with nocache=1 tacked on to the end stays put and doesn't get over written by the ending of drupal_goto.

This logic may very well be browser dependent. If it does the location redirect request with a POST then we are good & don't need the funky exit call in boost_exit. If the browser does a normal GET request with the new location then there is the potential for the message to not get through since the browser could get a cached page on the redirect. Let me know if you have any other questions on this subject.

Status: Fixed » Closed (fixed)

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

asak’s picture

Mikey, Torgos: Excellent stuff guys - highly appreciated!

Thanks for this great work.

joelstein’s picture

Just want to represent here that there are several modules which use hook_exit. In the application I'm building right now, there's Devel, LDAP Integration, and Organic Groups, all of which use hook_exit. It would be worth exploring if Boost can find a different way to do what it's doing in hook_exit.

torgosPizza’s picture

#70:

I've also found that Boost isn't compatible with the shURLy module (which allows you to turn your Drupal site into a URL shortening service) because it also utilizes hook_exit(). At this point I'm not sure if it's simply a module weight issue or something else. (In other words, would setting the Boost weight in the system table to 99 or something, to ensure it runs after everything else, solve these problems?)

It's worth discussing, or at least coming up with alternative solutions, I think.

joelstein’s picture

Status: Closed (fixed) » Active

If I'm not mistaken, Boost is actually set to be lighter than all the other modules, so that it can buffer the output and write the cached files.

Marking as "active", based on these recent comments, and #60.

torgosPizza’s picture

I disagree. This should be marked as fixed since the original issue (which was an Ubercart issue, sort of) was fixed in the patch, which was tested and committed. If the hook_exit() code in Boost needs to be refactored, we should start another issue for that.

mikeytown2’s picture

Plan A:
The 2 alts is to define language_url_rewrite OR custom_url_rewrite_outbound in order to add in the nocache=1 query string to the URL. If a site uses both then it would fallback to hook_exit.

Plan B:
Detect when a message is waiting to be sent for the user and set the drupal_uid cookie to something like -2. Once message is sent out then remove cookie.

torgosPizza’s picture

Sounds like Plan A is more reasonable... looking for other opinions as I could be wrong :)

mikeytown2’s picture

Plan A is the quick fix & but it could get ugly in the details (only mod URL if called from drupal_goto). Plan B is IMHO the better long term solution. It would be a modified version of what pressflow does with lazy session loading. Except in this case it would be a configurable instead of a catch all; default config would be to disable the boost cache if user has a messages in the session table. Once message has been delivered then re-enable boost cache.

chadcrew’s picture

I'm tracking this issue as I work on adding Boost to an Ubercart site. A couple of thoughts:

1) Wouldn't adding cart and cart/* to the "Cache every page except the listed pages" option in Boost solve this issue also? You'd never want those pages cached anyway.

2) To fix this issue more universally for Boost, would it make sense for boost_exit to run module_invoke_all('exit'), with a static variable test at the top to avoid a loop? Boost's weight means that it should run first, so it seems like that would let it do what it needs but still give all other modules a chance to run their exit functions.

I'm new to Boost though, so please correct me if my thinking is off-base!

Best,
Chad

torgosPizza’s picture

No, 1) does not fix the issue, because the hook_exit() causes Ubercart to never fire the add_to_cart function - it simply gets terminated prematurely before the Ubercart hooks have time to take effect. The form is the issue, not the cart page.

As far as #2 I'm not sure.. I'll defer to mikeytown2.

mikeytown2’s picture

I exclude the cart path from getting cached.
http://drupalcode.org/project/boost.git/blob/refs/heads/6.x-1.x:/boost.m...

option 2 is something to think about. The hook exit no cache redirect was code I inherited; there are better ways of doing this. I'm thinking about a 30 second no cache cookie or something to that effect. See plan B above. Sad to say but this is sorta low on my priority list. Getting 1.19 out is higher on that list.

Plan B code would look like this & would go in hook_exit; might need to be placed higher up.

if (!empty($_SESSION['messages']) && empty($_COOKIE['DRUPAL_UID'])) {
  // Set cookie if message needs to be sent out.
  boost_set_cookie(-2);
}
elseif (empty($_SESSION['messages']) && isset($_COOKIE['DRUPAL_UID']) && $_COOKIE['DRUPAL_UID'] == -2) {
  // Remove cookie.
  boost_set_cookie(0, BOOST_TIME - 86400);
}
chadcrew’s picture

FileSize
1.42 KB

Got it. Thanks for the clarification. In that case, the problem for ubercart isn't the missing hook_exit calls, it's the missing drupal_session_commit(). From drupal_goto:

  // Even though session_write_close() is registered as a shutdown function,
  // we need all session data written to the database before redirecting.
  drupal_session_commit();

So here's what I'm adding to boost_exit to fix the issue:

      // Do what drupal_goto() would do if we were to return to it:
      $boost_exit_running = TRUE;
      module_invoke_all('exit', $destination);
      drupal_session_commit();
      exit(header('Location: ' . $destination));

I've attached a patch versus the current stable release. (By the way, I'm currently testing on Pressflow 6 and Boost 6.x-1.18). This seems like a relatively clean way to fix Boost's current method.

As for plan B, at first glance (and if I understand what you're proposing) pausing caching for 30 seconds after a POST based on a cookie doesn't sound much cleaner to me. It's the time-based element that worries me -- if I have a problem downstream, now I have to worry about how far the user made it before caching came back on. Did they make it 2, 3, 4 more clicks? Reproducing downstream bugs could become a headache, no?

EDIT: sorry, my testing on Pressflow is throwing my patch off here. Standard D6 drupal_goto calls session_write_close(), but Pressflow calls drupal_session_commit(), due to it's lazy session handling. I think my solution still works, but it would need something like this to support both D6 and Pressflow 6:

if (function_exists('drupal_session_commit')) drupal_session_commit();
else session_write_close();
chadcrew’s picture

I think there might be another issue with boost_exit for ubercart:

Appending nocache=1 to the cart/checkout url can cause uc_cart's referrer check to fail. For example, enter an invalid coupon code (which causes a drupal_set_message call) on the checkout screen twice - on the second time, I lose all the info I've entered (shipping address, credit card, etc). Adding the following check seems to eliminate this issue:

if (stristr($destination, 'cart/checkout')) return;
mikeytown2’s picture

nocache=1 isn't a good solution IMHO. Messing with the cookie is a better solution.

wojtha’s picture

#79 PLAN B sounds promising

I'm now using boost together with Pressflow's Cookie cache bypass + I added following line to the .htaccess "boost skipping section":

  RewriteCond %{HTTP_COOKIE} NO_CACHE [OR]
ChrisLaFrancis’s picture

Subscribing.

achton’s picture

Subscribe.

com_net’s picture

Subscribe.
One who want to know how it doesn't work - can try http://legrand1.budka.ru

smira’s picture

confirming that torgosPizza's fix from #4/#6 works wonderfully for me patched against
uc-6.x-2.x-dev
pressflow 6.22
boost-6.x-1.8
does this have any chance of being committed or is it such an unique issue that we just patch and be done?
thank you all!

mikeytown2’s picture

Status: Active » Needs review

So #6 is the fix?

smira’s picture

the patch on #6 and .js file from #4 did it for me
so yeah basically #6 is the fix

3dloco’s picture

wouldn't #61 be the fix?

bgm’s picture

#61 worked for me (I upgraded to a 6.x-1.x dev snapshot of Boost).

#80 looks interesting, but I'm also using Pressflow and I don't fully understand the patch.

wojtha’s picture

Note that in #80 drupal_session_commit() is available since Drupal 7.x, for Drupal 6.x use session_write_close() instead.

wojtha’s picture

Status: Needs review » Needs work
m.stenta’s picture

What is the latest word on this? There have been a lot of changes to the dev branch since most of this issue's discussions.

I'm using the latest version (6.x-1.9-beta1) with Pressflow, and anonymous visitors cannot add products to their cart. It works fine with Boost disabled.

Edit: Nevermind! Sorry, I just found the "Exit() inside of boost_exit" setting. Turning it off (as recommended) fixes the issue for me. Thanks!

bgm’s picture

#61 is in 6.x-1.19-beta1. You have to enable the option in the boost settings ("do not exist" or something like that).

I'd be tempted to backport the solution in 7.x-1.x, which is to set the boost cookie to skip the cache for a single request. It removes the need to have a "nocache=1". See: #1242416: Avoid caching Drupal messages

sumit_kumar’s picture

Category: bug » support
Status: Needs work » Active

I think there might be another issue with boost_exit for ubercart:Appending nocache=1 to the cart/checkout url can cause uc_cart's referrer check to fail. For example, enter an invalid coupon code (which causes a drupal_set_message call) on the checkout screen twice - on the second time, I lose all the info I've entered (shipping address, credit card, etc). Adding the following check seems to eliminate this issue:

sip telephone service

Anonymous’s picture

Adding the following check seems to eliminate this issue

but I don't see a check attached (except for the user's signature).

bgm’s picture

Category: support » bug
Status: Active » Needs work

Please don't post spam on the issue queue.

The issue you describe in #96 is not related to the use of hook_exit(), please open a separate issue.

jvieille’s picture

Using Pressflow, the issue seemed to be solved no need to patch.
However, IE 11 fails : nothing in cart.
This new MS version is lightning fast, that might be related...

Eranga’s picture

I had the same issue in drupal 7 version. Disabling the secure pages module fixed the issue for me. I used .htaccess file for https instead of that module.