Error in CountryCacheContext if no country is set

I tested with 2.x but the fix should go into 3.x

If the error goes away without a store... Then I don't think this is worth troubleshooting since you can't do anything without stores on a regular Commerce installation... I see the page_cache module in the backtrace as well... You might consider uninstalling it.

@jsacksick: You mean "If the error goes away with a store"?

Yes, it does. But well, shouldn't we still simply return a fallback value then and leave a comment for that case?
I think that would make sense code-wise... and we're safe nobody will run into this. Even for tests it might be confusing, if you forgot to create a store or don't need it for that case?

My 2 cents... but I'll set priority to minor, definitely an edge-case!

Yes, I meant with a store.

But well, shouldn't we still simply return a fallback value then and leave a comment for that case?

What would be the fallback? The context has to return a string, we could fallback to US but at the sime... why the US as the default country? Unless it is fine to return a default country, I wonder what would be the implication of that change.

Unless it is fine to return a default country, I wonder what would be the implication of that change.

Not having people running into an unexpected null pointer error, from my perspective.

Maybe just return an empty string in that case? Or can we even tell the cache to not apply if it's empty?

With some comment lines, I think also the empty string or a fallback string might be fine for cache here? And I still think better than the null-pointer.

But of course you decide!

Did you try uninstalling the page_cache module? I'm just not sure why it'd trigger this whole logic... I think this logic shouldn't really be triggered.

Opened an MR (https://git.drupalcode.org/project/commerce/-/merge_requests/353/diffs), used "none" as the fallback, not sure that is the correct approach, but not sure what else we could do.

Thanks @jsacksick I'll give it a try, when I proceed with my work on commerce_giftcards in the next days!

If you can constantly reproduce the issue, you should be able to confirm quickly that the one liner fix is doing the trick.

Yeah I could, but I shut down that environment, on it again soon! :)

A test is failing, so this will NW.

I looked for a similar case in core and found https://api.drupal.org/api/drupal/core!modules!book!src!Cache!BookNaviga...

They do it similarly:

// If we're not looking at a book node, then we're not navigating a book.
    if ($current_bid === 0) {
        return 'book.none';
    }

So I think the 'none' fallback might be allright. Still I need to check #14 and we have to check why the test fails.

The failure is unrelated.

Branch was rebased and tests are passing now.

Thank you @jsacksick and sorry for being late, lots of work...

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