Merge lp:~blake-rouse/maas/fix-1484696 into lp:maas/trunk

Proposed by Blake Rouse on 2015-08-19
Status: Merged
Approved by: Blake Rouse on 2015-08-19
Approved revision: no longer in the source branch.
Merged at revision: 4206
Proposed branch: lp:~blake-rouse/maas/fix-1484696
Merge into: lp:maas/trunk
Diff against target: 135 lines (+23/-18)
2 files modified
src/maasserver/static/js/angular/factories/region.js (+4/-4)
src/maasserver/static/js/angular/factories/tests/test_region.js (+19/-14)
To merge this branch: bzr merge lp:~blake-rouse/maas/fix-1484696
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) 2015-08-19 Approve on 2015-08-19
Review via email: mp+268491@code.launchpad.net

Commit message

Regenerate the connection URL on websocket client reconnect.

Description of the change

I am really guessing at this one. This bug is really hard to reproduce no one really knows exactly how it is happening or what is causing it.

My guess here is that the websocket can connect faster than the cookie is updated when the entire page is loaded. So it will use the old csrftoken instead of the new one. If that happens then the retry will keep using that old token. This branch changes that so the connection url is regenerated on every connect so the new csrftoken is used.

To post a comment you must log in.
Andres Rodriguez (andreserl) wrote :

The branch overall looks good to me. I think we can give it a try in trunk and see what happens, hoewver, please prepare a backport bracnh to 1.8 so we can test it against 1.8 too.

review: Approve
lp:~blake-rouse/maas/fix-1484696 updated on 2015-08-19
4206. By Blake Rouse on 2015-08-19

[r=andreserl][bug=1484696][author=blake-rouse] Regenerate the connection URL on websocket client reconnect.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/static/js/angular/factories/region.js'
2--- src/maasserver/static/js/angular/factories/region.js 2015-06-26 12:47:19 +0000
3+++ src/maasserver/static/js/angular/factories/region.js 2015-08-19 15:22:53 +0000
4@@ -117,8 +117,8 @@
5 };
6
7 // Opens the websocket connection.
8- RegionConnection.prototype.connect = function(url) {
9- this.url = url;
10+ RegionConnection.prototype.connect = function() {
11+ this.url = this._buildUrl();
12 this.autoReconnect = true;
13 this.websocket = this.buildSocket(this.url);
14
15@@ -142,7 +142,7 @@
16 });
17 if(self.autoReconnect) {
18 $timeout(function() {
19- self.connect(self.url);
20+ self.connect();
21 }, self.retryTimeout);
22 }
23 };
24@@ -236,7 +236,7 @@
25 };
26 this.registerHandler("open", opened);
27 this.registerHandler("error", errored);
28- this.connect(this._buildUrl());
29+ this.connect();
30 return defer.promise;
31 };
32
33
34=== modified file 'src/maasserver/static/js/angular/factories/tests/test_region.js'
35--- src/maasserver/static/js/angular/factories/tests/test_region.js 2015-06-26 12:47:19 +0000
36+++ src/maasserver/static/js/angular/factories/tests/test_region.js 2015-08-19 15:22:53 +0000
37@@ -199,33 +199,37 @@
38
39 describe("connect", function() {
40
41- var url = "http://test-url";
42+ var url = "http://test-url", buildUrlSpy;
43+ beforeEach(function() {
44+ buildUrlSpy = spyOn(RegionConnection, "_buildUrl");
45+ buildUrlSpy.and.returnValue(url);
46+ });
47
48 it("sets url", function() {
49- RegionConnection.connect(url);
50+ RegionConnection.connect();
51 expect(RegionConnection.url).toBe(url);
52 });
53
54 it("sets autoReconnect to true", function() {
55 RegionConnection.autoReconnect = false;
56- RegionConnection.connect(url);
57+ RegionConnection.connect();
58 expect(RegionConnection.autoReconnect).toBe(true);
59 });
60
61 it("calls buildSocket with url", function() {
62- RegionConnection.connect(url);
63+ RegionConnection.connect();
64 expect(RegionConnection.buildSocket).toHaveBeenCalledWith(url);
65 });
66
67 it("sets websocket handlers", function() {
68- RegionConnection.connect(url);
69+ RegionConnection.connect();
70 expect(webSocket.onopen).not.toBeNull();
71 expect(webSocket.onerror).not.toBeNull();
72 expect(webSocket.onclose).not.toBeNull();
73 });
74
75 it("sets connect to true when onopen called", function() {
76- RegionConnection.connect(url);
77+ RegionConnection.connect();
78 webSocket.onopen({});
79 expect(RegionConnection.connected).toBe(true);
80 });
81@@ -236,13 +240,13 @@
82 expect(evt).toBe(evt_obj);
83 done();
84 });
85- RegionConnection.connect(url);
86+ RegionConnection.connect();
87 webSocket.onerror(evt_obj);
88 });
89
90 it("sets connect to false when onclose called", function() {
91 RegionConnection.autoReconnect = false;
92- RegionConnection.connect(url);
93+ RegionConnection.connect();
94 webSocket.onclose({});
95 expect(RegionConnection.connected).toBe(false);
96 });
97@@ -250,7 +254,7 @@
98 it("calls close handler when onclose called", function(done) {
99 var evt_obj = {};
100 RegionConnection.autoReconnect = false;
101- RegionConnection.connect(url);
102+ RegionConnection.connect();
103 RegionConnection.registerHandler("close", function(evt) {
104 expect(evt).toBe(evt_obj);
105 done();
106@@ -259,15 +263,16 @@
107 });
108
109 it("onclose calls connect when autoReconnect is true", function() {
110- RegionConnection.connect(url);
111- spyOn(RegionConnection, "connect");
112+ RegionConnection.connect();
113+ var new_url = "http://new-url";
114+ buildUrlSpy.and.returnValue(new_url);
115 webSocket.onclose({});
116 $timeout.flush();
117- expect(RegionConnection.connect).toHaveBeenCalledWith(url);
118+ expect(RegionConnection.url).toBe(new_url);
119 });
120
121 it("onclose sets error", function() {
122- RegionConnection.connect(url);
123+ RegionConnection.connect();
124 webSocket.onclose();
125 $timeout.flush();
126 expect(RegionConnection.error).toBe(
127@@ -277,7 +282,7 @@
128 it("calls onMessage when onmessage called", function() {
129 var sampleData = { sample: "data" };
130 spyOn(RegionConnection, "onMessage");
131- RegionConnection.connect(url);
132+ RegionConnection.connect();
133 webSocket.onmessage({ data: angular.toJson(sampleData) });
134 expect(RegionConnection.onMessage).toHaveBeenCalledWith(
135 sampleData);