Merge pull request #7820 from hashicorp/b-ui/ui-log-races

UI: Log streaming bug fix medley

ui/app/components/task-log.js

@@ -5,6 +5,12 @@ import RSVP from 'rsvp';
 import { logger } from 'nomad-ui/utils/classes/log';
 import timeout from 'nomad-ui/utils/timeout';
 
+class MockAbortController {
+  abort() {
+    /* noop */
+  }
+}
+
 export default Component.extend({
   token: service(),
 
@@ -45,12 +51,25 @@ export default Component.extend({
   logger: logger('logUrl', 'logParams', function logFetch() {
     // If the log request can't settle in one second, the client
     // must be unavailable and the server should be used instead
+
+    // 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), timeout(timing)]).then(
-        response => response,
+      RSVP.race([
+        this.token.authorizedRequest(url, { signal: aborter.signal }),
+        timeout(timing),
+      ]).then(
+        response => {
+          return response;
+        },
         error => {
-          if (this.useServer) {
+          aborter.abort();
+          if (useServer) {
             this.set('noConnection', true);
           } else {
             this.send('failoverToServer');
@@ -62,6 +81,7 @@ export default Component.extend({
 
   actions: {
     setMode(mode) {
+      if (this.mode === mode) return;
       this.logger.stop();
       this.set('mode', mode);
     },

ui/app/services/token.js

@@ -72,15 +72,16 @@ export default Service.extend({
   // This authorizedRawRequest is necessary in order to fetch data
   // with the guarantee of a token but without the automatic region
   // param since the region cannot be known at this point.
-  authorizedRawRequest(url, options = { credentials: 'include' }) {
+  authorizedRawRequest(url, options = {}) {
+    const credentials = 'include';
     const headers = {};
     const token = this.secret;
 
     if (token) {
       headers['X-Nomad-Token'] = token;
     }
 
-    return fetch(url, assign(options, { headers }));
+    return fetch(url, assign(options, { headers, credentials }));
   },
 
   authorizedRequest(url, options) {

ui/app/templates/components/task-log.hbs

@@ -1,7 +1,14 @@
 {{#if noConnection}}
   <div data-test-connection-error class="notification is-error">
-    <h3 class="title is-4">Cannot fetch logs</h3>
-    <p>The logs for this task are inaccessible. Check the condition of the node the allocation is on.</p>
+    <div class="columns">
+      <div class="column">
+        <h3 class="title is-4">Cannot fetch logs</h3>
+        <p>The logs for this task are inaccessible. Check the condition of the node the allocation is on.</p>
+      </div>
+      <div class="column is-centered is-minimum">
+        <button data-test-connection-error-dismiss class="button is-danger" onclick={{action (mut noConnection) false}}>Okay</button>
+      </div>
+    </div>
   </div>
 {{/if}}
 <div class="boxed-section-head">

ui/tests/integration/task-log-test.js

@@ -190,6 +190,25 @@ module('Integration | Component | task log', function(hooks) {
     );
   });
 
+  test('Clicking stderr/stdout mode buttons does nothing when the mode remains the same', async function(assert) {
+    const { interval } = commonProps;
+
+    run.later(() => {
+      click('[data-test-log-action="stdout"]');
+      run.later(run, run.cancelTimers, interval * 6);
+    }, interval * 2);
+
+    this.setProperties(commonProps);
+    await render(hbs`{{task-log allocation=allocation task=task}}`);
+
+    await settled();
+    assert.equal(
+      find('[data-test-log-cli]').textContent,
+      streamFrames[0] + streamFrames[0] + streamFrames[1],
+      'Now includes second frame'
+    );
+  });
+
   test('When the client is inaccessible, task-log falls back to requesting logs through the server', async function(assert) {
     run.later(run, run.cancelTimers, allowedConnectionTime * 2);
 
@@ -219,6 +238,11 @@ module('Integration | Component | task log', function(hooks) {
       this.server.handledRequests.filter(req => req.url.startsWith(serverUrl)).length,
       'Log request was later made to the server'
     );
+
+    assert.ok(
+      this.server.handledRequests.filter(req => clientUrlRegex.test(req.url))[0].aborted,
+      'Client log request was aborted'
+    );
   });
 
   test('When both the client and the server are inaccessible, an error message is shown', async function(assert) {
@@ -255,5 +279,53 @@ module('Integration | Component | task log', function(hooks) {
       'Log request was later made to the server'
     );
     assert.ok(find('[data-test-connection-error]'), 'An error message is shown');
+
+    await click('[data-test-connection-error-dismiss]');
+    assert.notOk(find('[data-test-connection-error]'), 'The error message is dismissable');
+  });
+
+  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 not shown');
   });
 });