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
=== modified file 'bus/connectivity/connectivity.go'
--- bus/connectivity/connectivity.go 2014-02-24 10:27:38 +0000
+++ bus/connectivity/connectivity.go 2014-03-20 12:39:26 +0000
@@ -145,7 +145,7 @@
145// The endpoint need not be dialed; connectivity will Dial() and Close()145// The endpoint need not be dialed; connectivity will Dial() and Close()
146// it as it sees fit.146// it as it sees fit.
147func ConnectedState(endp bus.Endpoint, config ConnectivityConfig, log logger.Logger, out chan<- bool) {147func ConnectedState(endp bus.Endpoint, config ConnectivityConfig, log logger.Logger, out chan<- bool) {
148 wg := NewWebchecker(config.ConnectivityCheckURL, config.ConnectivityCheckMD5, log)148 wg := NewWebchecker(config.ConnectivityCheckURL, config.ConnectivityCheckMD5, 10*time.Second, log)
149 cs := &connectedState{149 cs := &connectedState{
150 config: config,150 config: config,
151 log: log,151 log: log,
152152
=== modified file 'bus/connectivity/webchecker.go'
--- bus/connectivity/webchecker.go 2014-01-23 10:03:39 +0000
+++ bus/connectivity/webchecker.go 2014-03-20 12:39:26 +0000
@@ -26,8 +26,11 @@
26 "crypto/md5"26 "crypto/md5"
27 "fmt"27 "fmt"
28 "io"28 "io"
29 "time"
30
31 http13 "launchpad.net/ubuntu-push/http13client"
32
29 "launchpad.net/ubuntu-push/logger"33 "launchpad.net/ubuntu-push/logger"
30 "net/http"
31)34)
3235
33// how much web would a webchecker check36// how much web would a webchecker check
@@ -43,20 +46,25 @@
43 log logger.Logger46 log logger.Logger
44 url string47 url string
45 target string48 target string
49 cli *http13.Client
46}50}
4751
48// Build a webchecker for the given URL, that should match the target MD5.52// Build a webchecker for the given URL, that should match the target MD5.
49func NewWebchecker(url string, target string, log logger.Logger) Webchecker {53func NewWebchecker(url string, target string, timeout time.Duration, log logger.Logger) Webchecker {
50 return &webchecker{log, url, target}54 cli := &http13.Client{
55 Timeout: timeout,
56 Transport: &http13.Transport{TLSHandshakeTimeout: timeout},
57 }
58 return &webchecker{log, url, target, cli}
51}59}
5260
53// ensure webchecker implements Webchecker61// ensure webchecker implements Webchecker
54var _ Webchecker = &webchecker{}62var _ Webchecker = &webchecker{}
5563
56func (wb *webchecker) Webcheck(ch chan<- bool) {64func (wb *webchecker) Webcheck(ch chan<- bool) {
57 response, err := http.Get(wb.url)65 response, err := wb.cli.Get(wb.url)
58 if err != nil {66 if err != nil {
59 wb.log.Errorf("While GETting %s: %s", wb.url, err)67 wb.log.Errorf("While GETting %s: %v", wb.url, err)
60 ch <- false68 ch <- false
61 return69 return
62 }70 }
@@ -64,7 +72,7 @@
64 hash := md5.New()72 hash := md5.New()
65 _, err = io.CopyN(hash, response.Body, 1024)73 _, err = io.CopyN(hash, response.Body, 1024)
66 if err != io.EOF {74 if err != io.EOF {
67 wb.log.Errorf("Reading %s, expecting EOF, got: %s",75 wb.log.Errorf("Reading %s, expecting EOF, got: %v",
68 wb.url, err)76 wb.url, err)
69 ch <- false77 ch <- false
70 return78 return
7179
=== modified file 'bus/connectivity/webchecker_test.go'
--- bus/connectivity/webchecker_test.go 2014-02-05 18:17:26 +0000
+++ bus/connectivity/webchecker_test.go 2014-03-20 12:39:26 +0000
@@ -81,7 +81,7 @@
81 ts := httptest.NewServer(mkHandler(staticText))81 ts := httptest.NewServer(mkHandler(staticText))
82 defer ts.Close()82 defer ts.Close()
8383
84 ck := NewWebchecker(ts.URL, staticHash, s.log)84 ck := NewWebchecker(ts.URL, staticHash, 5*time.Second, s.log)
85 ch := make(chan bool, 1)85 ch := make(chan bool, 1)
86 ck.Webcheck(ch)86 ck.Webcheck(ch)
87 c.Check(<-ch, Equals, true)87 c.Check(<-ch, Equals, true)
@@ -89,7 +89,7 @@
8989
90// Webchecker sends false if the download fails.90// Webchecker sends false if the download fails.
91func (s *WebcheckerSuite) TestActualFails(c *C) {91func (s *WebcheckerSuite) TestActualFails(c *C) {
92 ck := NewWebchecker("garbage://", "", s.log)92 ck := NewWebchecker("garbage://", "", 5*time.Second, s.log)
93 ch := make(chan bool, 1)93 ch := make(chan bool, 1)
94 ck.Webcheck(ch)94 ck.Webcheck(ch)
95 c.Check(<-ch, Equals, false)95 c.Check(<-ch, Equals, false)
@@ -100,7 +100,7 @@
100 ts := httptest.NewServer(mkHandler(""))100 ts := httptest.NewServer(mkHandler(""))
101 defer ts.Close()101 defer ts.Close()
102102
103 ck := NewWebchecker(ts.URL, staticHash, s.log)103 ck := NewWebchecker(ts.URL, staticHash, 5*time.Second, s.log)
104 ch := make(chan bool, 1)104 ch := make(chan bool, 1)
105 ck.Webcheck(ch)105 ck.Webcheck(ch)
106 c.Check(<-ch, Equals, false)106 c.Check(<-ch, Equals, false)
@@ -111,7 +111,25 @@
111 ts := httptest.NewServer(mkHandler(bigText))111 ts := httptest.NewServer(mkHandler(bigText))
112 defer ts.Close()112 defer ts.Close()
113113
114 ck := NewWebchecker(ts.URL, bigHash, s.log)114 ck := NewWebchecker(ts.URL, bigHash, 5*time.Second, s.log)
115 ch := make(chan bool, 1)
116 ck.Webcheck(ch)
117 c.Check(<-ch, Equals, false)
118}
119
120// Webchecker sends false if the request timeouts
121func (s *WebcheckerSuite) TestTooSlowFails(c *C) {
122 finish := make(chan bool)
123 handler := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
124 <-finish // get stuck
125 })
126 ts := httptest.NewServer(handler)
127 defer ts.Close()
128 defer func() {
129 finish <- true
130 }()
131
132 ck := NewWebchecker(ts.URL, bigHash, time.Second, s.log)
115 ch := make(chan bool, 1)133 ch := make(chan bool, 1)
116 ck.Webcheck(ch)134 ck.Webcheck(ch)
117 c.Check(<-ch, Equals, false)135 c.Check(<-ch, Equals, false)

Subscribers

People subscribed via source and target branches