Merge pull request #9909 from hashicorp/b-ui/incorrect-error-message

UI: Use server-sent error messages when applicable

ui/app/components/job-page/parts/latest-deployment.js

@@ -1,6 +1,5 @@
 import Component from '@ember/component';
 import { task } from 'ember-concurrency';
-import { ForbiddenError } from '@ember-data/adapter/error';
 import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error';
 import { tagName } from '@ember-decorators/component';
 import classic from 'ember-classic-decorator';
@@ -18,15 +17,9 @@ export default class LatestDeployment extends Component {
     try {
       yield this.get('job.latestDeployment.content').promote();
     } catch (err) {
-      let message = messageFromAdapterError(err);
-
-      if (err instanceof ForbiddenError) {
-        message = 'Your ACL token does not grant permission to promote deployments.';
-      }
-
       this.handleError({
         title: 'Could Not Promote Deployment',
-        description: message,
+        description: messageFromAdapterError(err, 'promote deployments'),
       });
     }
   })

ui/app/components/job-page/parts/title.js

@@ -1,6 +1,5 @@
 import Component from '@ember/component';
 import { task } from 'ember-concurrency';
-import { ForbiddenError } from '@ember-data/adapter/error';
 import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error';
 import { tagName } from '@ember-decorators/component';
 import classic from 'ember-classic-decorator';
@@ -22,7 +21,7 @@ export default class Title extends Component {
     } catch (err) {
       this.handleError({
         title: 'Could Not Stop Job',
-        description: 'Your ACL token does not grant permission to stop jobs.',
+        description: messageFromAdapterError(err, 'stop jobs'),
       });
     }
   })
@@ -41,15 +40,9 @@ export default class Title extends Component {
       // Eagerly update the job status to avoid flickering
       job.set('status', 'running');
     } catch (err) {
-      let message = messageFromAdapterError(err);
-
-      if (err instanceof ForbiddenError) {
-        message = 'Your ACL token does not grant permission to stop jobs.';
-      }
-
       this.handleError({
         title: 'Could Not Start Job',
-        description: message,
+        description: messageFromAdapterError(err, 'start jobs'),
       });
     }
   })

ui/app/components/job-page/periodic.js

@@ -2,6 +2,7 @@ import AbstractJobPage from './abstract';
 import { inject as service } from '@ember/service';
 import { action } from '@ember/object';
 import classic from 'ember-classic-decorator';
+import messageForError from 'nomad-ui/utils/message-from-adapter-error';
 
 @classic
 export default class Periodic extends AbstractJobPage {
@@ -11,10 +12,10 @@ export default class Periodic extends AbstractJobPage {
 
   @action
   forceLaunch() {
-    this.job.forcePeriodic().catch(() => {
+    this.job.forcePeriodic().catch(err => {
       this.set('errorMessage', {
         title: 'Could Not Force Launch',
-        description: 'Your ACL token does not grant permission to submit jobs.',
+        description: messageForError(err, 'submit jobs'),
       });
     });
   }

ui/app/controllers/allocations/allocation/index.js

@@ -9,6 +9,7 @@ import { task } from 'ember-concurrency';
 import Sortable from 'nomad-ui/mixins/sortable';
 import { lazyClick } from 'nomad-ui/helpers/lazy-click';
 import { watchRecord } from 'nomad-ui/utils/properties/watch';
+import messageForError from 'nomad-ui/utils/message-from-adapter-error';
 import classic from 'ember-classic-decorator';
 
 @classic
@@ -73,7 +74,7 @@ export default class IndexController extends Controller.extend(Sortable) {
     } catch (err) {
       this.set('error', {
         title: 'Could Not Stop Allocation',
-        description: 'Your ACL token does not grant allocation lifecycle permissions.',
+        description: messageForError(err, 'manage allocation lifecycle'),
       });
     }
   })
@@ -85,7 +86,7 @@ export default class IndexController extends Controller.extend(Sortable) {
     } catch (err) {
       this.set('error', {
         title: 'Could Not Restart Allocation',
-        description: 'Your ACL token does not grant allocation lifecycle permissions.',
+        description: messageForError(err, 'manage allocation lifecycle'),
       });
     }
   })

ui/app/controllers/allocations/allocation/task/index.js

@@ -2,6 +2,7 @@ import Controller from '@ember/controller';
 import { computed as overridable } from 'ember-overridable-computed';
 import { task } from 'ember-concurrency';
 import classic from 'ember-classic-decorator';
+import messageForError from 'nomad-ui/utils/message-from-adapter-error';
 
 @classic
 export default class IndexController extends Controller {
@@ -21,7 +22,7 @@ export default class IndexController extends Controller {
     } catch (err) {
       this.set('error', {
         title: 'Could Not Restart Task',
-        description: 'Your ACL token does not grant allocation lifecycle permissions.',
+        description: messageForError(err, 'manage allocation lifecycle'),
       });
     }
   })

ui/app/utils/message-from-adapter-error.js

@@ -1,6 +1,14 @@
+import { ForbiddenError } from '@ember-data/adapter/error';
+
 // Returns a single string based on the response the adapter received
-export default function messageFromAdapterError(error) {
-  if (error.errors) {
+export default function messageFromAdapterError(error, actionMessage) {
+  if (error instanceof ForbiddenError) {
+    return `Your ACL token does not grant permission to ${actionMessage}.`;
+  }
+
+  if (error.errors?.length) {
     return error.errors.mapBy('detail').join('\n\n');
   }
+
+  return 'Unknown Error';
 }

ui/tests/acceptance/task-detail-test.js

@@ -223,7 +223,7 @@ module('Acceptance | task detail', function(hooks) {
     );
   });
 
-  test('when task restart fails, an error message is shown', async function(assert) {
+  test('when task restart fails (403), an ACL permissions error message is shown', async function(assert) {
     server.pretender.put('/v1/client/allocation/:id/restart', () => [403, {}, '']);
 
     await Task.restart.idle();
@@ -241,6 +241,22 @@ module('Acceptance | task detail', function(hooks) {
     assert.notOk(Task.inlineError.isShown, 'Inline error is no longer shown');
   });
 
+  test('when task restart fails (500), the error message from the API is piped through to the alert', async function(assert) {
+    const message = 'A plaintext error message';
+    server.pretender.put('/v1/client/allocation/:id/restart', () => [500, {}, message]);
+
+    await Task.restart.idle();
+    await Task.restart.confirm();
+
+    assert.ok(Task.inlineError.isShown);
+    assert.ok(Task.inlineError.title.includes('Could Not Restart Task'));
+    assert.equal(Task.inlineError.message, message);
+
+    await Task.inlineError.dismiss();
+
+    assert.notOk(Task.inlineError.isShown);
+  });
+
   test('exec button is present', async function(assert) {
     assert.ok(Task.execButton.isPresent);
   });

ui/tests/unit/utils/message-from-adapter-error-test.js

@@ -0,0 +1,44 @@
+import { module, test } from 'qunit';
+import { ServerError, ForbiddenError } from '@ember-data/adapter/error';
+import messageFromAdapterError from 'nomad-ui/utils/message-from-adapter-error';
+
+const testCases = [
+  {
+    name: 'Forbidden Error',
+    in: [new ForbiddenError([], "Can't do that"), 'run tests'],
+    out: 'Your ACL token does not grant permission to run tests.',
+  },
+  {
+    name: 'Generic Error',
+    in: [new ServerError([{ detail: 'DB Max Connections' }], 'Server Error'), 'run tests'],
+    out: 'DB Max Connections',
+  },
+  {
+    name: 'Multiple Errors',
+    in: [
+      new ServerError(
+        [{ detail: 'DB Max Connections' }, { detail: 'Service timeout' }],
+        'Server Error'
+      ),
+      'run tests',
+    ],
+    out: 'DB Max Connections\n\nService timeout',
+  },
+  {
+    name: 'Malformed Error (not from Ember Data which should always have an errors list)',
+    in: [new Error('Go boom'), 'handle custom error messages'],
+    out: 'Unknown Error',
+  },
+];
+
+module('Unit | Util | messageFromAdapterError', function() {
+  testCases.forEach(testCase => {
+    test(testCase.name, function(assert) {
+      assert.equal(
+        messageFromAdapterError.apply(null, testCase.in),
+        testCase.out,
+        `[${testCase.in.join(', ')}] => ${testCase.out}`
+      );
+    });
+  });
+});