Repository: favonia/cloudflare-ddns (go)

Issue:
Re-probe 1.1.1.1 versus 1.0.0.1 if earlier probing failed
(Inspired by the discussion at #706.) When

1. It is meaningful to check 1.1.1.1 v.s. 1.0.0.1 and
2. All prior probing failed (we failing to probe both 1.1.1.1 and 1.0.0.1) and
3. We are about to detect potentially new IP addresses

Re-check 1.1.1.1 v.s. 1.0.0.1!

Pull request description:
feat: recheck 1.1.1.1 versus 1.0.0.1 later when probing fails
Close #721.

Patch:
diff --git a/cmd/ddns/ddns.go b/cmd/ddns/ddns.go
index 6be4c6b..ac3f645 100644
--- a/cmd/ddns/ddns.go
+++ b/cmd/ddns/ddns.go
@@ -32,7 +32,7 @@ func initConfig(ctx context.Context, ppfmt pp.PP) (*config.Config, setter.Setter
 	c := config.Default()
 
 	// Read the config
-	if !c.ReadEnv(ppfmt) || !c.NormalizeConfig(ppfmt) || !c.ShouldWeUse1001(ctx, ppfmt) {
+	if !c.ReadEnv(ppfmt) || !c.NormalizeConfig(ppfmt) {
 		return c, nil, false
 	}
 
diff --git a/internal/config/config.go b/internal/config/config.go
index 757862f..579141c 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -18,7 +18,7 @@ import (
 type Config struct {
 	Auth             api.Auth
 	Provider         map[ipnet.Type]provider.Provider
-	Use1001          bool
+	ShouldWeUse1001  *bool
 	Domains          map[ipnet.Type][]domain.Domain
 	UpdateCron       cron.Schedule
 	UpdateOnStart    bool
@@ -42,7 +42,7 @@ func Default() *Config {
 			ipnet.IP4: provider.NewCloudflareTrace(),
 			ipnet.IP6: provider.NewCloudflareTrace(),
 		},
-		Use1001: false,
+		ShouldWeUse1001: nil,
 		Domains: map[ipnet.Type][]domain.Domain{
 			ipnet.IP4: nil,
 			ipnet.IP6: nil,
diff --git a/internal/config/network_probe.go b/internal/config/network_probe.go
index cab1fef..500343d 100644
--- a/internal/config/network_probe.go
+++ b/internal/config/network_probe.go
@@ -27,14 +27,15 @@ func ProbeURL(ctx context.Context, url string) bool {
 	return err == nil
 }
 
-// ShouldWeUse1001 quickly checks 1.1.1.1 and 1.0.0.1 and notes whether 1.0.0.1 should be used.
+// ShouldWeUse1001Now quickly checks 1.1.1.1 and 1.0.0.1 and notes whether 1.0.0.1 should be used.
 //
-// Note that the return value is about whether the detection is successfully done, not that
-// whether we should use 1.0.0.1. The function will update the field [Config.Use1001] directly.
-func (c *Config) ShouldWeUse1001(ctx context.Context, ppfmt pp.PP) bool {
-	c.Use1001 = false
+// [Config.ShouldWeUse1001] remembers the results.
+func (c *Config) ShouldWeUse1001Now(ctx context.Context, ppfmt pp.PP) bool {
+	if c.ShouldWeUse1001 != nil {
+		return *c.ShouldWeUse1001
+	}
 	if c.Provider[ipnet.IP4] == nil || !c.Provider[ipnet.IP4].ShouldWeCheck1111() {
-		return true
+		return false // any answer would work
 	}
 
 	if ppfmt.IsEnabledFor(pp.Info) {
@@ -44,16 +45,22 @@ func (c *Config) ShouldWeUse1001(ctx context.Context, ppfmt pp.PP) bool {
 
 	if ProbeURL(ctx, "https://1.1.1.1") {
 		ppfmt.Infof(pp.EmojiGood, "1.1.1.1 is working. Great!")
+
+		res := false
+		c.ShouldWeUse1001 = &res
+		return false
 	} else {
 		if ProbeURL(ctx, "https://1.0.0.1") {
 			ppfmt.Warningf(pp.EmojiError, "1.1.1.1 is not working, but 1.0.0.1 is; using 1.0.0.1")
 			ppfmt.Infof(pp.EmojiHint, "1.1.1.1 is probably blocked or hijacked by your router or ISP")
-			c.Use1001 = true
+
+			res := true
+			c.ShouldWeUse1001 = &res
+			return true
 		} else {
-			ppfmt.Warningf(pp.EmojiError, "Both 1.1.1.1 and 1.0.0.1 are not working; sticking to 1.1.1.1")
-			ppfmt.Infof(pp.EmojiHint, "The network might be temporarily down, or has not been set up yet")
-			ppfmt.Infof(pp.EmojiHint, "If you start this tool during booting, make sure the network is already up")
+			ppfmt.Warningf(pp.EmojiError, "Both 1.1.1.1 and 1.0.0.1 are not working; sticking to 1.1.1.1 now")
+			ppfmt.Infof(pp.EmojiHint, "The network might be temporarily down; will redo probing later")
+			return false
 		}
 	}
-	return true
 }
diff --git a/internal/updater/updater.go b/internal/updater/updater.go
index ad8475f..2cd95a0 100644
--- a/internal/updater/updater.go
+++ b/internal/updater/updater.go
@@ -141,7 +141,8 @@ func UpdateIPs(ctx context.Context, ppfmt pp.PP, c *config.Config, s setter.Sett
 
 	for _, ipNet := range [...]ipnet.Type{ipnet.IP4, ipnet.IP6} {
 		if c.Provider[ipNet] != nil {
-			ip, msg := detectIP(ctx, ppfmt, c, ipNet, c.Use1001)
+			use1001 := c.ShouldWeUse1001Now(ctx, ppfmt)
+			ip, msg := detectIP(ctx, ppfmt, c, ipNet, use1001)
 			msgs = append(msgs, msg)
 
 			// Note: If we can't detect the new IP address,


Test patch:
diff --git a/internal/config/network_probe_test.go b/internal/config/network_probe_test.go
index 2d036db..a0195fa 100644
--- a/internal/config/network_probe_test.go
+++ b/internal/config/network_probe_test.go
@@ -48,8 +48,9 @@ func TestProbeCloudflareIPs(t *testing.T) {
 	)
 	c := config.Default()
 	// config.ShouldWeUse1001 must return false on GitHub.
-	require.True(t, c.ShouldWeUse1001(context.Background(), mockPP))
-	require.False(t, c.Use1001)
+	require.False(t, c.ShouldWeUse1001Now(context.Background(), mockPP))
+	require.NotNil(t, c.ShouldWeUse1001)
+	require.False(t, *c.ShouldWeUse1001)
 }
 
 func TestProbeCloudflareIPsNoIP4(t *testing.T) {
@@ -58,6 +59,6 @@ func TestProbeCloudflareIPsNoIP4(t *testing.T) {
 	mockPP := mocks.NewMockPP(mockCtrl)
 	c := config.Default()
 	c.Provider[ipnet.IP4] = nil
-	require.True(t, c.ShouldWeUse1001(context.Background(), mockPP))
-	require.False(t, c.Use1001)
+	require.False(t, c.ShouldWeUse1001Now(context.Background(), mockPP))
+	require.Nil(t, c.ShouldWeUse1001)
 }
diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go
index f680feb..d9c8f25 100644
--- a/internal/updater/updater_test.go
+++ b/internal/updater/updater_test.go
@@ -103,7 +103,8 @@ func TestUpdateIPsMultiple(t *testing.T) {
 				domain4_4: false,
 			}
 			conf.RecordComment = RecordComment
-			conf.Use1001 = true
+			use1001 := true
+			conf.ShouldWeUse1001 = &use1001
 			conf.DetectionTimeout = time.Second
 			conf.UpdateTimeout = time.Second
 			mockPP := mocks.NewMockPP(mockCtrl)
@@ -203,7 +204,8 @@ func TestDeleteIPsMultiple(t *testing.T) {
 				domain4_4: false,
 			}
 			conf.RecordComment = RecordComment
-			conf.Use1001 = true
+			use1001 := true
+			conf.ShouldWeUse1001 = &use1001
 			conf.DetectionTimeout = time.Second
 			conf.UpdateTimeout = time.Second
 			mockPP := mocks.NewMockPP(mockCtrl)
@@ -280,7 +282,8 @@ func TestUpdateIPsUninitializedProxied(t *testing.T) {
 			conf.TTL = api.TTLAuto
 			conf.Proxied = map[domain.Domain]bool{}
 			conf.RecordComment = RecordComment
-			conf.Use1001 = true
+			use1001 := true
+			conf.ShouldWeUse1001 = &use1001
 			conf.DetectionTimeout = time.Second
 			conf.UpdateTimeout = time.Second
 			mockPP := mocks.NewMockPP(mockCtrl)
@@ -369,7 +372,8 @@ func TestUpdateIPsHints(t *testing.T) {
 			conf.TTL = api.TTLAuto
 			conf.Proxied = map[domain.Domain]bool{domain4: false, domain6: false}
 			conf.RecordComment = RecordComment
-			conf.Use1001 = true
+			use1001 := true
+			conf.ShouldWeUse1001 = &use1001
 			conf.DetectionTimeout = time.Second
 			conf.UpdateTimeout = time.Second
 			mockPP := mocks.NewMockPP(mockCtrl)
@@ -657,7 +661,8 @@ func TestUpdateIPs(t *testing.T) {
 			conf.TTL = api.TTLAuto
 			conf.Proxied = map[domain.Domain]bool{domain4: false, domain6: false}
 			conf.RecordComment = RecordComment
-			conf.Use1001 = true
+			use1001 := true
+			conf.ShouldWeUse1001 = &use1001
 			conf.DetectionTimeout = time.Second
 			conf.UpdateTimeout = time.Second
 			mockPP := mocks.NewMockPP(mockCtrl)
@@ -823,7 +828,8 @@ func TestDeleteIPs(t *testing.T) {
 			conf.TTL = api.TTLAuto
 			conf.Proxied = map[domain.Domain]bool{domain4: false, domain6: false}
 			conf.RecordComment = RecordComment
-			conf.Use1001 = true
+			use1001 := true
+			conf.ShouldWeUse1001 = &use1001
 			conf.DetectionTimeout = time.Second
 			conf.UpdateTimeout = time.Second
 			mockPP := mocks.NewMockPP(mockCtrl)