Ubuntu Push Notifications

Merge lp:~pedronis/ubuntu-push/webchecker-http-timeout into lp:ubuntu-push

  1. webchecker-http-timeout
  2. Merge into trunk
Proposed by Samuele Pedroni
Status: Merged
Approved by: Samuele Pedroni
Approved revision: 93
Merged at revision: 83
Proposed branch: lp:~pedronis/ubuntu-push/webchecker-http-timeout
Merge into: lp:ubuntu-push
Prerequisite: lp:~pedronis/ubuntu-push/http13client
Diff against target: 126 lines (+37/-11)
3 files modified
bus/connectivity/connectivity.go (+1/-1)
bus/connectivity/webchecker.go (+14/-6)
bus/connectivity/webchecker_test.go (+22/-4)
To merge this branch: bzr merge lp:~pedronis/ubuntu-push/webchecker-http-timeout
Reviewer Review Type Date Requested Status
John Lenton (community) Approve
Review via email: mp+211921@code.launchpad.net

Commit message

add timeing out to webhecker using the vendorized go 1.3 net/http in http13client

Description of the change

add timeing out to webhecker using the vendorized go 1.3 net/http in http13client,

the added test would previously never end

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) wrote :

Excellent, thank you!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bus/connectivity/connectivity.go'
2--- bus/connectivity/connectivity.go 2014-02-24 10:27:38 +0000
3+++ bus/connectivity/connectivity.go 2014-03-20 12:39:26 +0000
4@@ -145,7 +145,7 @@
5 // The endpoint need not be dialed; connectivity will Dial() and Close()
6 // it as it sees fit.
7 func ConnectedState(endp bus.Endpoint, config ConnectivityConfig, log logger.Logger, out chan<- bool) {
8- wg := NewWebchecker(config.ConnectivityCheckURL, config.ConnectivityCheckMD5, log)
9+ wg := NewWebchecker(config.ConnectivityCheckURL, config.ConnectivityCheckMD5, 10*time.Second, log)
10 cs := &connectedState{
11 config: config,
12 log: log,
13
14=== modified file 'bus/connectivity/webchecker.go'
15--- bus/connectivity/webchecker.go 2014-01-23 10:03:39 +0000
16+++ bus/connectivity/webchecker.go 2014-03-20 12:39:26 +0000
17@@ -26,8 +26,11 @@
18 "crypto/md5"
19 "fmt"
20 "io"
21+ "time"
22+
23+ http13 "launchpad.net/ubuntu-push/http13client"
24+
25 "launchpad.net/ubuntu-push/logger"
26- "net/http"
27 )
28
29 // how much web would a webchecker check
30@@ -43,20 +46,25 @@
31 log logger.Logger
32 url string
33 target string
34+ cli *http13.Client
35 }
36
37 // Build a webchecker for the given URL, that should match the target MD5.
38-func NewWebchecker(url string, target string, log logger.Logger) Webchecker {
39- return &webchecker{log, url, target}
40+func NewWebchecker(url string, target string, timeout time.Duration, log logger.Logger) Webchecker {
41+ cli := &http13.Client{
42+ Timeout: timeout,
43+ Transport: &http13.Transport{TLSHandshakeTimeout: timeout},
44+ }
45+ return &webchecker{log, url, target, cli}
46 }
47
48 // ensure webchecker implements Webchecker
49 var _ Webchecker = &webchecker{}
50
51 func (wb *webchecker) Webcheck(ch chan<- bool) {
52- response, err := http.Get(wb.url)
53+ response, err := wb.cli.Get(wb.url)
54 if err != nil {
55- wb.log.Errorf("While GETting %s: %s", wb.url, err)
56+ wb.log.Errorf("While GETting %s: %v", wb.url, err)
57 ch <- false
58 return
59 }
60@@ -64,7 +72,7 @@
61 hash := md5.New()
62 _, err = io.CopyN(hash, response.Body, 1024)
63 if err != io.EOF {
64- wb.log.Errorf("Reading %s, expecting EOF, got: %s",
65+ wb.log.Errorf("Reading %s, expecting EOF, got: %v",
66 wb.url, err)
67 ch <- false
68 return
69
70=== modified file 'bus/connectivity/webchecker_test.go'
71--- bus/connectivity/webchecker_test.go 2014-02-05 18:17:26 +0000
72+++ bus/connectivity/webchecker_test.go 2014-03-20 12:39:26 +0000
73@@ -81,7 +81,7 @@
74 ts := httptest.NewServer(mkHandler(staticText))
75 defer ts.Close()
76
77- ck := NewWebchecker(ts.URL, staticHash, s.log)
78+ ck := NewWebchecker(ts.URL, staticHash, 5*time.Second, s.log)
79 ch := make(chan bool, 1)
80 ck.Webcheck(ch)
81 c.Check(<-ch, Equals, true)
82@@ -89,7 +89,7 @@
83
84 // Webchecker sends false if the download fails.
85 func (s *WebcheckerSuite) TestActualFails(c *C) {
86- ck := NewWebchecker("garbage://", "", s.log)
87+ ck := NewWebchecker("garbage://", "", 5*time.Second, s.log)
88 ch := make(chan bool, 1)
89 ck.Webcheck(ch)
90 c.Check(<-ch, Equals, false)
91@@ -100,7 +100,7 @@
92 ts := httptest.NewServer(mkHandler(""))
93 defer ts.Close()
94
95- ck := NewWebchecker(ts.URL, staticHash, s.log)
96+ ck := NewWebchecker(ts.URL, staticHash, 5*time.Second, s.log)
97 ch := make(chan bool, 1)
98 ck.Webcheck(ch)
99 c.Check(<-ch, Equals, false)
100@@ -111,7 +111,25 @@
101 ts := httptest.NewServer(mkHandler(bigText))
102 defer ts.Close()
103
104- ck := NewWebchecker(ts.URL, bigHash, s.log)
105+ ck := NewWebchecker(ts.URL, bigHash, 5*time.Second, s.log)
106+ ch := make(chan bool, 1)
107+ ck.Webcheck(ch)
108+ c.Check(<-ch, Equals, false)
109+}
110+
111+// Webchecker sends false if the request timeouts
112+func (s *WebcheckerSuite) TestTooSlowFails(c *C) {
113+ finish := make(chan bool)
114+ handler := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
115+ <-finish // get stuck
116+ })
117+ ts := httptest.NewServer(handler)
118+ defer ts.Close()
119+ defer func() {
120+ finish <- true
121+ }()
122+
123+ ck := NewWebchecker(ts.URL, bigHash, time.Second, s.log)
124 ch := make(chan bool, 1)
125 ck.Webcheck(ch)
126 c.Check(<-ch, Equals, false)

Subscribers

People subscribed via source and target branches