Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Analyze @throws tags #31329

Open
luisfarzati opened this issue May 8, 2019 · 11 comments
Open

Feature: Analyze @throws tags #31329

luisfarzati opened this issue May 8, 2019 · 11 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@luisfarzati
Copy link

Description

Given a function has a @throws JSDoc tag, it would be helpful to apply some analysis and raise a compiler warning (or, if configured, an error) when the caller does not handle the error and it doesn't declare its own @throws tag.

Examples

/**
 * @throws {SomeException}
 */
function myFunction() {
  // some logic
  // then for some reason we throw an exception
  if (somethingWentWrong) {
    throw new SomeException("something happened");
  }
  // some more logic
}

function potentialMess() {
  myFunction(); // Compiler error: 'myFunction' may throw `SomeException`. ts(9876)
}

/**
 * @throws {SomeException}
 */
function letItBubble() {
  myFunction(); // No compiler errors
}

function aFunctionThatHandlesTheException() {
  try {
    myFunction(); // No compiler errors
  }
  catch (error) {
    // do something with the error
  }
}

Questions

  • Ideally, it would be even better if VSCode realizes that myFunction throws an error, and then @throws is not required for the analysis. Still, this approach would be useful when dealing with external code/interfaces.

  • It is my understanding that JSDoc's @throws is allowed once -- if this is correct, then VSCode could ignore this and accept multiple tags anyway? Otherwise, maybe we can use a different tag, e.g. @exception?

@luisfarzati luisfarzati changed the title Feature Request: Analyze @throws hints Feature Request: Analyze @throws tags May 8, 2019
@vscodebot vscodebot bot assigned mjbvz May 8, 2019
@mjbvz mjbvz transferred this issue from microsoft/vscode May 9, 2019
@mjbvz
Copy link
Contributor

mjbvz commented May 9, 2019

This is likely out of scope. TypeScript powers the VS Code JavaScript IntelliSense and it does not have support for java typed exceptions like this describes. I also do not think this it is on the roadmap

@mjbvz mjbvz removed their assignment May 9, 2019
@luisfarzati
Copy link
Author

luisfarzati commented May 11, 2019

Thanks @mjbvz for the response. Given that other common JSDoc tags such as @type, @typedef, @param, @returns have been implemented and are currently supported in VS Code and type checking, is @throws really that different to be considered out of scope? Or is it because of the efforts required to achieve it?

Update: actually, when it comes to parsing JSDoc, on second thought I believe this suggestion should be done to the VS Code project, sorry. Still, would be nice to see TypeScript support for typed errors, but this is not what this issue describes.

Thank you!

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels May 13, 2019
@AlCalzone
Copy link
Contributor

I second this. Having support for throws in TypeScript would be nice aswell, so we could mark which functions are unsafe to call without wrapping the call in try/catch.

The syntax could be something along the lines of

function dooSomething(): void | throws {
}

@dragomirtitian
Copy link
Contributor

Not sure @ahejlsberg has changed his stance on checked exceptions:

Anders Hejlsberg: ... The concern I have about checked exceptions is the handcuffs they put on programmers. You see programmers picking up new APIs that have all these throws clauses, and then you see how convoluted their code gets, and you realize the checked exceptions aren't helping them any. It is sort of these dictatorial API designers telling you how to do your exception handling. They should not be doing that.

Source

I personally agree with this, I have seen a lot of bad code written as a result of people being forced to handle exceptions they have no idea what to do with (ending up in exceptions being eaten without a trace and the app not working)

@AlCalzone not sure just throws would be enough .. any function could throw a random run-time exception it would be interesting to know what it throws and handle only that, but without multiple typed catch clauses (which would be new expression level syntax) I'm not sure how you can ensure an exception type is handled.

@nrei0
Copy link

nrei0 commented May 19, 2019

+1

Let me show you some code, which shows the real case.

import { Channel, Game } from '../game';
import { GeneralError, errorCode } from '../error';

/**
 * @typedef { import('../game/channel_storage').ChannelStorage } ChannelStorage
 * @typedef { import('../game/game').Game } Game
 */

/**
 * Host game action.
 *
 * @param {Object} o Options.
 * @param {ChannelStorage} o.channelStorage Game channel storage.
 * @param {Number} o.maxPlayers Max players in a game.
 * @param {String} o.channelId Discord channel id.
 * @returns {Game} Hosted game.
 * @throws {GeneralError} General error.
 */
export const hostGame = ({ channelStorage, maxPlayers, channelId }) => {
  const channel = channelStorage.registerChannel(new Channel(channelId, { maxGames: 1 }));

  if (!channel) {
    throw new GeneralError(
      "Couldn't register a new game channel",
      errorCode.CHANNEL_ALREADY_EXISTED
    );
  }

  const game = channel.registerGame(new Game({ maxPlayers }));

  if (!game) {
    throw new GeneralError(
      "Couldn't register a new game in the channel",
      errorCode.GAME_ALREADY_EXISTED
    );
  }

  return game;
};

So, seems very useful to have @throws {GeneralError}.
Actually, in the case of multiple throws, you can write per line:

@throws {GeneralError} Throws in A, B, C cases.
@throws {WhateverError} Throws whatever case met.

In the case, if you are using 3rd party functions in your own implementation with their own throws, you can also write typedef import for this specific error and write extra throws JSDoc for your function.

@luisfarzati
Copy link
Author

I sure see @ahejlsberg's point, but at the same time I believe it would be a very useful feature even in the example he puts, since especially when dealing with new APIs you may not know what kind of exceptions (or when) this API can throw.

This could be implemented as another strict rule that you can opt-in, so everybody's happy.

@luisfarzati luisfarzati changed the title Feature Request: Analyze @throws tags Feature: Analyze @throws tags May 20, 2019
@nrei0
Copy link

nrei0 commented May 20, 2019

I can understand the argument against not using throws in JSDoc only because of dynamic nature.

For example

try { myFunc() } catch (e) {} // what is `e` here? We don't know even.

But, if I will highlight myFunc will be great to go JSDoc and see multiple throws declarations with a way to overview the definition. Then I will understand what exactly I would handle and how.

Example

try { myFunc } catch (e) { if (e instanceOf ConnectionError) { console.log(e.prettyCustomLog()) }   }

I know about ConnectionError and props without spending hours for debugging cases, I just read JSDoc and look into definition of that error object.

* @throws {ConnectionError} In a case if connection try was failed.
* @throws {DBWriteError} In a case if failed to write DB

@jhpratt
Copy link

jhpratt commented Jun 12, 2019

For what it's worth, I'd very much like some sort of indicator that a function can throw. I just tried extracting out a common "unexpected state" throw, and realized that I can't do so safely because TS lacks this functionality.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 12, 2019

A potential compromise is for JS, that @throws is evaluated as a function that returns never of which, there is a level of checking built in to ensure that such a function doesn't return a value.

@vultix
Copy link

vultix commented Aug 29, 2019

For those that are looking for compile-time error safety in typescript you can use my ts-results library.

@ExE-Boss
Copy link
Contributor

This will need #13219 to be implemented first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

10 participants