Skip to content

Commit

Permalink
fix: use new 'instanceOf' in the catch blocks (twilio#99)
Browse files Browse the repository at this point in the history
Fixes the use of `instanceof` by recursively checking the instance constructor name.
  • Loading branch information
ktalebian authored Jul 22, 2020
1 parent 2de7aff commit 1cffe58
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 6 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ twilio-cli-*.tgz

### Backup package.json files created during release ###
*.bak

.idea
5 changes: 3 additions & 2 deletions src/base-commands/base-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const { logger, LoggingLevel } = require('../services/messaging/logging');
const { OutputFormats } = require('../services/output-formats');
const { getCommandPlugin, requireInstall } = require('../services/require-install');
const { SecureStorage } = require('../services/secure-storage');
const { instanceOf } = require('../services/javascript-utilities');
let inquirer; // We'll lazy-load this only when it's needed.

const DEFAULT_LOG_LEVEL = 'info';
Expand Down Expand Up @@ -53,11 +54,11 @@ class BaseCommand extends Command {
}

async catch(error) {
if (!this.logger || error instanceof CLIError) {
if (!this.logger || instanceOf(error, CLIError)) {
return super.catch(error);
}

if (error instanceof TwilioCliError) {
if (instanceOf(error, TwilioCliError)) {
// User/API errors
this.logger.error(error.message);
this.logger.debug(error.stack);
Expand Down
4 changes: 2 additions & 2 deletions src/base-commands/twilio-client-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const BaseCommand = require('./base-command');
const CliRequestClient = require('../services/cli-http-client');
const { TwilioApiClient, TwilioApiFlags } = require('../services/twilio-api');
const { TwilioCliError } = require('../services/error');
const { translateValues } = require('../services/javascript-utilities');
const { translateValues, instanceOf } = require('../services/javascript-utilities');
const { camelCase, kebabCase } = require('../services/naming-conventions');
const { ACCESS_DENIED, HELP_ENVIRONMENT_VARIABLES } = require('../services/messaging/help-messages');

Expand Down Expand Up @@ -56,7 +56,7 @@ class TwilioClientCommand extends BaseCommand {
async catch(error) {
// Append to the error message when catching API access denied errors with
// profile-auth (i.e., standard API key auth).
if (error instanceof TwilioCliError && error.exitCode === ACCESS_DENIED_CODE) {
if (instanceOf(error, TwilioCliError) && error.exitCode === ACCESS_DENIED_CODE) {
if (!this.currentProfile.id.startsWith('${TWILIO')) { // Auth *not* using env vars.
error.message += '\n\n' + ACCESS_DENIED;
}
Expand Down
25 changes: 24 additions & 1 deletion src/services/javascript-utilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,34 @@ const splitArray = (array, testFunc) => {
return [left, right];
};

/**
* Checks whether an object is instance of a given class
* @param {Object} instance the instance object to check
* @param klass the class the instance object to be checked against
* @returns {boolean} whether the instance is instanceof the provided klass
*/
const instanceOf = (instance, klass) => {
while (instance && instance !== Object.prototype) {
if (!instance || !instance.constructor || !instance.constructor.name) {
return false;
}

if (klass.name === instance.constructor.name) {
return true;
}

instance = Object.getPrototypeOf(instance);
}

return false;
};

module.exports = {
doesObjectHaveProperty,
translateObject,
translateKeys,
translateValues,
sleep,
splitArray
splitArray,
instanceOf
};
34 changes: 33 additions & 1 deletion test/services/javascript-utilities.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { doesObjectHaveProperty, translateKeys, translateValues, sleep, splitArray } = require('../../src/services/javascript-utilities');
const { doesObjectHaveProperty, translateKeys, translateValues, sleep, splitArray, instanceOf } = require('../../src/services/javascript-utilities');

const { expect, test } = require('@twilio/cli-test');

Expand Down Expand Up @@ -120,5 +120,37 @@ describe('services', () => {
expect(notMatched).to.deep.equal(['ey!', 'bee', 'SEA']);
});
});

describe('instanceOf', () => {
class BaseError extends Error {
// No-op
}

class ExtendedError extends BaseError {
// No-op
}

test.it('should return true for instanceOf', () => {
const baseError = new BaseError();
const extendedError = new ExtendedError();

expect(instanceOf(extendedError, ExtendedError)).to.equal(true);
expect(instanceOf(extendedError, BaseError)).to.equal(true);
expect(instanceOf(extendedError, Error)).to.equal(true);

expect(instanceOf(baseError, BaseError)).to.equal(true);
expect(instanceOf(baseError, Error)).to.equal(true);
});

test.it('should return false for instanceOf', () => {
class Foo extends Error {}

const baseError = new BaseError();
const extendedError = new ExtendedError();

expect(instanceOf(baseError, Foo)).to.equal(false);
expect(instanceOf(extendedError, Foo)).to.equal(false);
});
});
});
});

0 comments on commit 1cffe58

Please sign in to comment.