mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
127 stars 41 forks source link

[Bug]: Missing block type in cache.json results in non interable data set #15163

Closed KevinMind closed 1 week ago

KevinMind commented 1 week ago

What happened?

See: https://mozilla.sentry.io/issues/6060337908/?project=6237717&referrer=slack

Here is precisely where the problem occurs. We load the cache.json and pick keys off of it. when we attempt to pick soft_blocked it is not defined on the json and so get returns None We don't have tests for this case because we always generate each block type.. but the current iteration of the cron is comparing against a cache that was generated before we introduced soft block into the data set.. Once we pass this iteration.. the bug wouldn't happen again.. but it could happen if for any reason a key is missing.. so fallback to empty list actually solves the problem. Makes sense?

What did you expect to happen?

If for any reason we are missing a key in cache.json we should fallback to using an empty array for the data set.

The only other alternative I could consider is we assume the filter is totally useless and regenerate from scratch. I think this could be overkill though, since we always produce the full cache.json and should expect each block type to be present.

Is there an existing issue for this?

┆Issue is synchronized with this Jira Task

ioanarusiczki commented 1 week ago

Re-checking what's been tested yesterday, I've 2024-11-13T17:11:14.387Z at blocklists/addons-bloomfilters in remote settings.

AMO Dev

The bucket shows all guids https://remote-settings-dev.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records

{73dac385-1afb-45c5-97a9-050237ff00be} https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/634085 blocked {b95304d3-8159-428c-9dfa-e7ccb442d38f} https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/634086 blocked {eba7d82d-98fe-400c-8346-0e96db72824f} 1.0 https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/634084 blocked

So this looks good.

AMO stage

Bucket is empty https://firefox.settings.services.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records

But I see the guids have been blocked except for a soft block (which is expected)

{7a9e3f2d-f415-43c8-bb83-e26f708e74fa} https://reviewers.addons.allizom.org/en-US/reviewers/review/1032169 {5ba93104-ecf8-4e5e-909e-14856e2dc663} https://reviewers.addons.allizom.org/en-US/reviewers/review/1032170 {7b99e6ec-b8ac-4dda-b716-b36b58bf13a0} https://reviewers.addons.allizom.org/en-US/reviewers/review/1032171

blocked

I'll setup again some blocks on stage, see about the bucket too.

willdurand commented 1 week ago

Bucket is empty firefox.settings.services.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records

This shows a non empty bucket for me now, there is the base bloom filter in there.

ioanarusiczki commented 1 week ago

Idk, I see this as I'm writing it down

Dev - with guids dev

Stage stage

willdurand commented 1 week ago

Yeah so dev has a few stash records (and likely a base bloom filter too), while stage only has the base bloom filter.

ioanarusiczki commented 1 week ago

Stage looks good

blocked and unblocked 3 guids , 2 extensions , one theme

stash https://firefox.settings.services.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records?_changeset=0

{4d9d078b-a3af-4c09-8d70-4f64918e3f04} {652b92a0-0a2b-42b0-bb9f-cb5554beef8a} {8e345df4-728b-486b-9cac-fea8d8088663}

I don't think it's a problem but just in case, after the last update in remote-settings I've a warning displayed next to bloomfilters (this was triggered manually for the delete block test). I'll keep an eye on this when I'll try again blocking.

remote settings warning