Skip to content

Commit

Permalink
Fix race condition where stdout and stderr requests can cause a no co…
Browse files Browse the repository at this point in the history
…nnection error

This would happen because a no connection error happens after the second request fails, but
that's because it's assumed the second request is to a server node. However, if a user clicks
stderr fast enough, the first and second requests are both to the client node. This changes
the logic to check if the request is to the server before deeming log streaming a total failure.
  • Loading branch information
DingoEatingFuzz committed Apr 28, 2020
1 parent ddfe167 commit 185891f
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
6 changes: 5 additions & 1 deletion ui/app/components/task-log.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export default Component.extend({
// AbortControllers don't exist in IE11, so provide a mock if it doesn't exist
const aborter = window.AbortController ? new AbortController() : new MockAbortController();
const timing = this.useServer ? this.serverTimeout : this.clientTimeout;

// Capture the state of useServer at logger create time to avoid a race
// between the stdout logger and stderr logger running at once.
const useServer = this.useServer;
return url =>
RSVP.race([
this.token.authorizedRequest(url, { signal: aborter.signal }),
Expand All @@ -65,7 +69,7 @@ export default Component.extend({
},
error => {
aborter.abort();
if (this.useServer) {
if (useServer) {
this.set('noConnection', true);
} else {
this.send('failoverToServer');
Expand Down
45 changes: 45 additions & 0 deletions ui/tests/integration/task-log-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,49 @@ module('Integration | Component | task log', function(hooks) {
);
assert.ok(find('[data-test-connection-error]'), 'An error message is shown');
});

test('When the client is inaccessible, the server is accessible, and stderr is pressed before the client timeout occurs, the no connection error is not shown', async function(assert) {
// override client response to timeout
this.server.get(
`http://${HOST}/v1/client/fs/logs/:allocation_id`,
() => [400, {}, ''],
allowedConnectionTime * 2
);

// Click stderr before the client request responds
run.later(() => {
click('[data-test-log-action="stderr"]');
run.later(run, run.cancelTimers, commonProps.interval * 5);
}, allowedConnectionTime / 2);

this.setProperties(commonProps);
await render(hbs`{{task-log
allocation=allocation
task=task
clientTimeout=clientTimeout
serverTimeout=serverTimeout}}`);

await settled();

const clientUrlRegex = new RegExp(`${HOST}/v1/client/fs/logs/${commonProps.allocation.id}`);
const clientRequests = this.server.handledRequests.filter(req => clientUrlRegex.test(req.url));
assert.ok(
clientRequests.find(req => req.queryParams.type === 'stdout'),
'Client request for stdout'
);
assert.ok(
clientRequests.find(req => req.queryParams.type === 'stderr'),
'Client request for stderr'
);

const serverUrl = `/v1/client/fs/logs/${commonProps.allocation.id}`;
assert.ok(
this.server.handledRequests
.filter(req => req.url.startsWith(serverUrl))
.find(req => req.queryParams.type === 'stderr'),
'Server request for stderr'
);

assert.notOk(find('[data-test-connection-error]'), 'An error message is shown');
});
});

0 comments on commit 185891f

Please sign in to comment.