twilio-video.js icon indicating copy to clipboard operation
twilio-video.js copied to clipboard

TwilioError is undefined

Open mrlika opened this issue 2 years ago • 5 comments

  • [x] I have verified that the issue occurs with the latest twilio-video.js release and is not marked as a known issue in the CHANGELOG.md.
  • [x] I reviewed the Common Issues and open GitHub issues and verified that this report represents a potentially new issue.
  • [x] I verified that the Quickstart application works in my environment.
  • [x] I am not sharing any Personally Identifiable Information (PII) or sensitive account information (API keys, credentials, etc.) when reporting this issue.

Code to reproduce the issue:

import { TwilioError  } from 'twilio-video';

console.log(TwilioError);
'x' instanceof TwilioError;

Expected behavior:

TwilioError to be a class 'x' instanceof TwilioError should not throw an exception

Actual behavior:

TwilioError is undefined 'x' instanceof TwilioError causes an exception

Software versions:

  • [x] twilio-video.js: 2.21.1
  • [x] React

I think TwilioError declared in types

https://github.com/twilio/twilio-video.js/blob/352ebf3e1d85aacdf4a888ba8a99e4ab37465131/tsdef/index.d.ts#L44

is actually not exported here:

https://github.com/twilio/twilio-video.js/blob/352ebf3e1d85aacdf4a888ba8a99e4ab37465131/lib/index.ts#L69

mrlika avatar Apr 20 '22 11:04 mrlika

@mrlika thanks for reporting. The definition for TwilioError is not coming from lib/index.ts. It is coming from tsdef/TwilioError which is in the same directory as index.d.ts, the line you referenced. We are able to use it fine in our React App. Please see this https://github.com/twilio/twilio-video-app-react/search?q=TwilioError

Can you please tell me how are you including twilio-video in your project (npm, cdn, github, local dir)?

charliesantos avatar Apr 20 '22 21:04 charliesantos

Everything is fine with TwilioError as a definition.

But TwilioError is not only a type (definition) but it is also a class (i.e. a construction function). It should be possible to use this class for example as a right-hand parameter of instanceof operator.

Definitions say that this TwilioError class (i.e. constructor function) is exported by twilio-video module but actually it is not and TwilioVideo is undefined.

And the following constructions don't work because of this (but should work according definitions):


new TwilioError('Some error'); // throws exception that TwilioError is undefined

// or

try{
  // ....
} catch (e) {
   if (e instanceof TwilioError) { // trhows exception that right-hand parameter is undefined
      // do something
   }
}

mrlika avatar Apr 21 '22 05:04 mrlika

@mrlika that's a valid point, however, we currently don't allow instantiation of TwilioError outside of our SDK so there is no need for us to export it.

Can you please explain your use case or scenario? Is it just trying to check if the exception is an instance of TwilioError?

charliesantos avatar Apr 21 '22 15:04 charliesantos

There are multiple cases with that:

  1. twilio-video definitions are incorrect because they argue that TwilioError is a JavaScript class (i.e. constructor function): https://github.com/twilio/twilio-video.js/blob/352ebf3e1d85aacdf4a888ba8a99e4ab37465131/tsdef/TwilioError.d.ts#L1 This is the root issue. It can be defined as a type or an interface if it is not a class. But defining errors as types or interfaces is not common practice in JavaScript world so anyway it will be confusing.
  2. We rely on TypeScript and definitions to create reliable code and catch as many as possible issues while writing code, not in run-time. It saves a lot of money, decreases development time, and gives many other benefits. Because of this, our developer wrote valid according to the definitions code if (e instanceof TwilioError) console.log(e.code); that pass through all the TypeScript, linter, and code review. It cost us about 1 day of developer time to find and fix the issue because it appeared for a client, then went through support to a developer. Then it required time to analyze and find this very tricky bug in twilio-video definitions.
  3. You can google for javascript error handling best practices and in most of articles you can find usage of instanceof operator to distinguish between error types. So this bug will waste more and more money for your customers with time...

mrlika avatar Apr 21 '22 20:04 mrlika

Hey @mrlika thank you for the additional information. I understand the use case. I submitted a ticket internally to address this issue.

charliesantos avatar Apr 24 '22 23:04 charliesantos