salarcode / SmartProxy

Firefox/Chrome browser extension. SmartProxy will automatically enable/disable proxy for the sites you visit, based on customizable patterns.
https://addons.mozilla.org/en-US/firefox/addon/smartproxy/
GNU General Public License v3.0
1.93k stars 117 forks source link

Stops working if can't re-fetch proxy server subscription #394

Closed WofWca closed 1 month ago

WofWca commented 2 months ago

Description

Even after fixing #393 I am still observing that if the proxy list fails to refresh (e.g. it gets blocked, or goes down), the locally stored list of proxies gets cleared, and the extension stops working.

I really think it ought to be the case that the list should not be cleared if a fetch fails, same way as won't let you "re-save" the proxy subscription in settings if the URL is not available.

Steps to Reproduce

  1. Add proxy server list subscription
  2. Set proxy mode to "Always enabled"
  3. Set a proxy from that list as the default proxy
  4. Somehow ensure that the proxy list cannot be fetched anymore. Perhaps you could try using a local server to serve the list of proxies, but don't forget to open dev tools and disable cache.
  5. Restart the browser.

You'll see that the extension stopped proxying, and if you go to "Proxy Servers" settings, you'll see that "Default Proxy Server" is empty and there are no servers you can select.

Which browsers did you test this on?

Other (Chromium based)

Affected browser versions

Chrome 128

Affected SmartProxy versions

v1.5

Screenshots of the problems or steps to reproduce

image

Any additional context

I suspect this has to do with this line:

https://github.com/salarcode/SmartProxy/blob/ba303196da5e1161bece745b645f4554f400a22e/src/core/SettingsOperation.ts#L91

when it is called here:

https://github.com/salarcode/SmartProxy/blob/33ba1df1eb32d14d389d0b4c10e364f65af55823/src/core/Settings.ts#L87

This was introduced in b138fcb547ef11cb5826ad9c7a09aa4f0404970f This happens on every service worker restart. What is the point of removing all proxies from the subscription? Shouldn't this line just be removed.

salarcode commented 1 month ago

Thanks for the report, this makes sense. The code was added to make sure we do have an array object.

The logic is updated now