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
=== modified file 'src/maasserver/static/js/angular/factories/region.js'
--- src/maasserver/static/js/angular/factories/region.js 2015-06-26 12:47:19 +0000
+++ src/maasserver/static/js/angular/factories/region.js 2015-08-19 15:22:53 +0000
@@ -117,8 +117,8 @@
117 };117 };
118118
119 // Opens the websocket connection.119 // Opens the websocket connection.
120 RegionConnection.prototype.connect = function(url) {120 RegionConnection.prototype.connect = function() {
121 this.url = url;121 this.url = this._buildUrl();
122 this.autoReconnect = true;122 this.autoReconnect = true;
123 this.websocket = this.buildSocket(this.url);123 this.websocket = this.buildSocket(this.url);
124124
@@ -142,7 +142,7 @@
142 });142 });
143 if(self.autoReconnect) {143 if(self.autoReconnect) {
144 $timeout(function() {144 $timeout(function() {
145 self.connect(self.url);145 self.connect();
146 }, self.retryTimeout);146 }, self.retryTimeout);
147 }147 }
148 };148 };
@@ -236,7 +236,7 @@
236 };236 };
237 this.registerHandler("open", opened);237 this.registerHandler("open", opened);
238 this.registerHandler("error", errored);238 this.registerHandler("error", errored);
239 this.connect(this._buildUrl());239 this.connect();
240 return defer.promise;240 return defer.promise;
241 };241 };
242242
243243
=== modified file 'src/maasserver/static/js/angular/factories/tests/test_region.js'
--- src/maasserver/static/js/angular/factories/tests/test_region.js 2015-06-26 12:47:19 +0000
+++ src/maasserver/static/js/angular/factories/tests/test_region.js 2015-08-19 15:22:53 +0000
@@ -199,33 +199,37 @@
199199
200 describe("connect", function() {200 describe("connect", function() {
201201
202 var url = "http://test-url";202 var url = "http://test-url", buildUrlSpy;
203 beforeEach(function() {
204 buildUrlSpy = spyOn(RegionConnection, "_buildUrl");
205 buildUrlSpy.and.returnValue(url);
206 });
203207
204 it("sets url", function() {208 it("sets url", function() {
205 RegionConnection.connect(url);209 RegionConnection.connect();
206 expect(RegionConnection.url).toBe(url);210 expect(RegionConnection.url).toBe(url);
207 });211 });
208212
209 it("sets autoReconnect to true", function() {213 it("sets autoReconnect to true", function() {
210 RegionConnection.autoReconnect = false;214 RegionConnection.autoReconnect = false;
211 RegionConnection.connect(url);215 RegionConnection.connect();
212 expect(RegionConnection.autoReconnect).toBe(true);216 expect(RegionConnection.autoReconnect).toBe(true);
213 });217 });
214218
215 it("calls buildSocket with url", function() {219 it("calls buildSocket with url", function() {
216 RegionConnection.connect(url);220 RegionConnection.connect();
217 expect(RegionConnection.buildSocket).toHaveBeenCalledWith(url);221 expect(RegionConnection.buildSocket).toHaveBeenCalledWith(url);
218 });222 });
219223
220 it("sets websocket handlers", function() {224 it("sets websocket handlers", function() {
221 RegionConnection.connect(url);225 RegionConnection.connect();
222 expect(webSocket.onopen).not.toBeNull();226 expect(webSocket.onopen).not.toBeNull();
223 expect(webSocket.onerror).not.toBeNull();227 expect(webSocket.onerror).not.toBeNull();
224 expect(webSocket.onclose).not.toBeNull();228 expect(webSocket.onclose).not.toBeNull();
225 });229 });
226230
227 it("sets connect to true when onopen called", function() {231 it("sets connect to true when onopen called", function() {
228 RegionConnection.connect(url);232 RegionConnection.connect();
229 webSocket.onopen({});233 webSocket.onopen({});
230 expect(RegionConnection.connected).toBe(true);234 expect(RegionConnection.connected).toBe(true);
231 });235 });
@@ -236,13 +240,13 @@
236 expect(evt).toBe(evt_obj);240 expect(evt).toBe(evt_obj);
237 done();241 done();
238 });242 });
239 RegionConnection.connect(url);243 RegionConnection.connect();
240 webSocket.onerror(evt_obj);244 webSocket.onerror(evt_obj);
241 });245 });
242246
243 it("sets connect to false when onclose called", function() {247 it("sets connect to false when onclose called", function() {
244 RegionConnection.autoReconnect = false;248 RegionConnection.autoReconnect = false;
245 RegionConnection.connect(url);249 RegionConnection.connect();
246 webSocket.onclose({});250 webSocket.onclose({});
247 expect(RegionConnection.connected).toBe(false);251 expect(RegionConnection.connected).toBe(false);
248 });252 });
@@ -250,7 +254,7 @@
250 it("calls close handler when onclose called", function(done) {254 it("calls close handler when onclose called", function(done) {
251 var evt_obj = {};255 var evt_obj = {};
252 RegionConnection.autoReconnect = false;256 RegionConnection.autoReconnect = false;
253 RegionConnection.connect(url);257 RegionConnection.connect();
254 RegionConnection.registerHandler("close", function(evt) {258 RegionConnection.registerHandler("close", function(evt) {
255 expect(evt).toBe(evt_obj);259 expect(evt).toBe(evt_obj);
256 done();260 done();
@@ -259,15 +263,16 @@
259 });263 });
260264
261 it("onclose calls connect when autoReconnect is true", function() {265 it("onclose calls connect when autoReconnect is true", function() {
262 RegionConnection.connect(url);266 RegionConnection.connect();
263 spyOn(RegionConnection, "connect");267 var new_url = "http://new-url";
268 buildUrlSpy.and.returnValue(new_url);
264 webSocket.onclose({});269 webSocket.onclose({});
265 $timeout.flush();270 $timeout.flush();
266 expect(RegionConnection.connect).toHaveBeenCalledWith(url);271 expect(RegionConnection.url).toBe(new_url);
267 });272 });
268273
269 it("onclose sets error", function() {274 it("onclose sets error", function() {
270 RegionConnection.connect(url);275 RegionConnection.connect();
271 webSocket.onclose();276 webSocket.onclose();
272 $timeout.flush();277 $timeout.flush();
273 expect(RegionConnection.error).toBe(278 expect(RegionConnection.error).toBe(
@@ -277,7 +282,7 @@
277 it("calls onMessage when onmessage called", function() {282 it("calls onMessage when onmessage called", function() {
278 var sampleData = { sample: "data" };283 var sampleData = { sample: "data" };
279 spyOn(RegionConnection, "onMessage");284 spyOn(RegionConnection, "onMessage");
280 RegionConnection.connect(url);285 RegionConnection.connect();
281 webSocket.onmessage({ data: angular.toJson(sampleData) });286 webSocket.onmessage({ data: angular.toJson(sampleData) });
282 expect(RegionConnection.onMessage).toHaveBeenCalledWith(287 expect(RegionConnection.onMessage).toHaveBeenCalledWith(
283 sampleData);288 sampleData);