search

interledger / web-monetization-extension

An open-source browser extension that enables Web Monetization.
Apache License 2.0
46 stars 2 forks source link

fix: use window ID when querying active tab #333

Closed raducristianpopa closed 6 days ago

raducristianpopa commented 1 week ago

Context

Part of #292.

There was another edge case, but the video is not available anymore. It can be fixed in another PR or we keep this as draft until then.

Changes proposed in this pull request

Aparently this is a known bug: https://issues.chromium.org/issues/41160154.

In the video that was provided in the initial issue, when the extension popup is open and the focus is on the DevTools. moving focus to the extension's popup, we will get an empty array when we query the active tab.

The false positive is happening because of this return statement: https://github.com/interledger/web-monetization-extension/blob/bf1baff5802a4472045c1bcca094873b3f4b919a/src/background/services/monetization.ts#L201-L203

Any suggestions for a good error message?

raducristianpopa commented 1 week ago

Extension builds preview

Name Link
Latest commit 29cd7f44d67a2a19d3076b203ca43562b69f0171
Latest job logs Run #9591811519
BadgeDownload
BadgeDownload
raducristianpopa commented 1 week ago

Any suggestions for a good error message in case this is going to happen again? The false positive was happening because we did not throw an error when the tab ID was not defined, here:

https://github.com/interledger/web-monetization-extension/blob/740821ab1633fa14ac6125b433afa13ff81e3c82/src/background/services/monetization.ts#L203

@sidvishnoi @dianafulga

sidvishnoi commented 1 week ago

Re https://github.com/interledger/web-monetization-extension/pull/333#issuecomment-2178059932

Does this happen only when devtools are open? When the tab.id may be undefined?

raducristianpopa commented 1 week ago

Re #333 (comment)

Does this happen only when devtools are open? When the tab.id may be undefined?

At one point I was able to reproduce it when switching between different window applications (another browser window or another app).

sidvishnoi commented 1 week ago

@raducristianpopa I could not reproduce this. Maybe we can add a simple error like "Could not find active tab" so we know what caused it in future.

sidvishnoi commented 1 week ago

Unrelated/related: I get this randomly sometimes image

sessions undefined for some reason (just for additional safety, that check should be on sessions?.size, so we don't accidentally get divide by zero error. https://github.com/interledger/web-monetization-extension/blob/740821ab1633fa14ac6125b433afa13ff81e3c82/src/background/services/monetization.ts#L207-L212