Jump to content
thirty bees forum

Update to 1.0.8 Shows 500 Server Error


violinparts

Recommended Posts

You can ignore that error. The problem is bellow that message:

[ThirtyBeesException] [ PSCURRENCYDEFAULT] is not a valid configuration key at line 1005 in file classes/Configuration.php

    • @since 1.0.8
  1. */
  2. protected static function validateKey($key)
  3. {
  4. if ( ! Validate::isConfigName($key)) {
  5. $e = new PrestaShopException(sprintf(
  6. Tools::displayError('[%s] is not a valid configuration key'),
  7. Tools::htmlentitiesUTF8($key)
  8. ));
  9. die($e->displayMessage());
  10. }
Link to comment
Share on other sites

The problem is not in the tb core, it's in the elasticsearch module, on line 484:

$defaultCurrency = Currency::getCurrencyInstance(Configuration::get(' PS_CURRENCY_DEFAULT'));

The intent of code above is to return currency object for default currency. But it actually never did, because of the typo in configuration key. The Configuration::get(' PS_CURRENCY_DEFAULT') statement returned null because ' PS_CURRENCY_DEFAULT' does not exists (there exists only key without leading space) That in turn resulted in Currency::getCurrencyInstance(null)call, which returned brand new, unsaved currency.

As you can see, everything's wrong. The rest of the code continued to evaluate with wrong currency object. We don't know what went wrong down the stream, but rest assured -- this definitely manifested in some other bugs later on.

Thirtybees 1.0.8 introduced new check that actually detects these kind of bugs. I agree that the die was too much, simple warning in log might have been enough. But then again -- nobody really notice these warnings in logs, right?

To fix your problem, simply replace the line with

$defaultCurrency = Currency::getCurrencyInstance(Configuration::get('PS_CURRENCY_DEFAULT'));

  • Thanks 1
Link to comment
Share on other sites

@Briljander handling invalid inputs is a really hard problem for us developers. What should we know when we know for sure that the input is incorrect? When we know that the caller made a mistake, that there is more definitely a bug? Shall we try to recover somehow, or just give up?

From my personal experience, recovering does not usually work very well. That's because we don't know the context, what the caller expects, etc.

For example, take a division by zero. I have seen many programs that just bails the caller out ty returning zero in that case. You must agree that's not a good solution, since it will most likely cause other problems later. What if the returned number is used to multiply the cart total? The total result would be zero as well, and merchants could quite easily loose real world money because of such bug.

It's very similar in this case. TB core doesn't know what the caller use the result for. But we know that such key can not exists in the db. Can we simply return null? Since we don't know what the result is used for, it would be a real gamble. Again, this might very easily cost you money.

I'm a big fan of fail fast philosophy in these cases.

I understand that, in this particular case, it would be probably better for @violinparts if thirtybees just returned null. His site would work as before, and he wouldn't have to solve this crisis. But by failing fast we have discovered and fixed another bug in the code, making the whole product more stable for everyone

Link to comment
Share on other sites

@datakick I cannot agree with this. Will it help to discover bugs? Sure. Does it justify to turn down a site. NOT AT ALL. ZERO. You have to find another method, if not medium-big merchants will run away. This is a horrible approach. Enable notifications, even enable a checkbox to send you the error. The bug is huge? well, the site owner will know there is a bug and he will decide to run or not during you fix that bug. Time will be the same (if TB takes care of the bug) and you do not put the merchant into the stress of a falling site (in some cases, that can cause thousands of dollars). I will be really clear and honest. I love TB, I have been here since almost the start, and my product will be launched in 2019. If this philosophy of "falling down to discover and repair" continues as soon as it makes me site falling and I lost orders, I will change the platform. Not only for the money and brand image (if you have high traffic can be a huge impact) but also because I do not want to pass over the stress that a fallen site means.

Link to comment
Share on other sites

@rubben1985 this philosophy is not new, and it's used in every software I know. It's called exception handling, and the gist is simple - if the code discovers an error, and can't safely recover from it, then it should throw the exception and transfer the responsibility to the caller. That's the best way to deal with this problem I know of, and I've been developing software for 20 years now.

When we throw an exception in the code, we do it because we really don't know how to solve the situation at hand. Imagine if you received an email order for product you don't sell. Would you just send different product to the customer?... Because this is what you are asking thirtybees to do -- to ignore the problem and carry on like noting happened. But something happened, we know that much.

Link to comment
Share on other sites

@datakick this does raise a good point though. I do tend to fall on the side of not breaking shops, but you are very right in that we lose the ability to track and trace errors too.

What about a data collection method? What if we just started logging all fatal errors that are catchable in a phone home type server? Just about every executable software company does that these days.

Link to comment
Share on other sites

@datakick said in Update to 1.0.8 Shows 500 Server Error:

@rubben1985 this philosophy is not new, and it's used in every software I know. It's called exception handling, and the gist is simple - if the code discovers an error, and can't safely recover from it, then it should throw the exception and transfer the responsibility to the caller. That's the best way to deal with this problem I know of, and I've been developing software for 20 years now.

When we throw an exception in the code, we do it because we really don't know how to solve the situation at hand. Imagine if you received an email order for product you don't sell. Would you just send different product to the customer?... Because this is what you are asking thirtybees to do -- to ignore the problem and carry on like noting happened. But something happened, we know that much.

@datakick but you can just send a message to yourself, or inform the client (if that is technically possible). No necessary to fall down the web. This only creates a bad image of the platform. In addittion you will earn almost nothing because if that happens, the owner, the first thing it will do (unless traffic is tooooo low) is going to be to restore backup immediately. When he restores, we will have a bad impression, fear to update again, and you will not have the error to analyze it because it is already restored. I can not find the good side in a production environment to be honest

Link to comment
Share on other sites

I just reported the same [ PSCURRENCYDEFAULT] is not a valid configuration key at line 1005 in file classes/Configuration.php error in the Dutch forum. I have attached the module - a Dutch carrier module - file there. The error happens at the frontside when there is something in the shopping cart.

https://forum.thirtybees.com/topic/2402/myparcel-met-tb-1-08-geeft-problemen

I don't use Elastic Search

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...