typescript use of try catch and async await

Question

Hello I would like to know if I am using async await and promise correctly, basically I am using a lot of try catches and it seems to me that all these try catches are not necessary

i have this main class:

export class BootstrapApplication {
  private server: WebServer;
  private knex: KnexInstance;
  constructor() {
    this.server = new ExpressServer();
    this.knex = container.resolve(KnexInstance);
  }
  public start = async () => {
    try {
      await this.knex.start(knexConfig);
      await this.server.start();
    } catch (error) {
      throw error;
    }
  };
}

and start server with this:

(async () => {
  const server = new BootstrapApplication();
  await server.start();
})();

and I have in the bootstrap class a function to start where I call my knex.start and my knex start which is asynchronous has 3 asynchronous functions that are using try catch:

@singleton()
export class KnexInstance {
  private knex: Knex;
  public get Knex() {
    return this.knex;
  }
  private assertDatabaseConnection = () => {
    return new Promise(resolve => {
      this.knex
        .raw('select 1+1 as result')
        .then(() => resolve())
        .catch(err => {
          console.log(
            '[Fatal] Failed to establish connection to database! Exiting...',
          );
          console.log(err);
          process.exit(1);
        });
    });
  };
  private runMigrations = async () => {
    try {
      console.log('Migration started...');
      await this.knex.migrate.latest({ loadExtensions: ['.ts'] });
      console.log('Migration finished...');
    } catch (err) {
      throw err;
    }
  };
  public start = async (config: Knex.Config) => {
    if (this.knex instanceof Knex) return;
    try {
      this.knex = Knex(config);
      await this.runMigrations();
      await this.assertDatabaseConnection();
    } catch (error) {
      throw error;
    }
  };
}

so i am in doubt if i am using async await in the wrong way

Well, `catch(e) { throw e }` kinda "noop" :) Also you don't really need `new Promise` constructor (ever) — Yury Tarabanko, Oct 07 '20 at 14:56
if I understand correctly: catch (error) {throw error} is unnecessary? and is it not correct to use the new Promise? and when I want to make a promise like I would? — Ming, Oct 07 '20 at 15:02
If you only want to rethrow the error, than try-catch is not necessary. New Promise is an antipattern https://stackoverflow.com/a/23803744/351705. If you can't handle the error in place you should not use try-catch and postpone error handling to the calling site. — Yury Tarabanko, Oct 07 '20 at 15:37
I’m voting to close this question because it belongs to codereview.stackexchange.com — Yury Tarabanko, Oct 07 '20 at 15:38
@YuryTarabanko While this may be on-topic on CR, in the future, please don't use the existence of the Code Review site as a reason to close a question. Evaluate the request and use a reason like *needs focus*, *primarily opinion-based*, etc. Then you can mention to the OP that it can be posted on Code Review if it is [on-topic](https://codereview.stackexchange.com/help/on-topic). Please see [_Does being on-topic at another Stack Exchange site automatically make a question off-topic for Stack Overflow?_](https://meta.stackoverflow.com/q/287400/1575353) — Sᴀᴍ Onᴇᴌᴀ, Oct 07 '20 at 16:13
I apologize I thought that because it is linked to code it could be posted on stackoverflow. — Ming, Oct 07 '20 at 21:13

Tim Perry · Accepted Answer · 2020-10-07T16:20:02.097

try {
  // ...
} catch (e) {
  throw e;
}

^ This is always a no-op that does nothing, in both normal and async functions.

You're right - on your example code, you can safely remove all of those structures.

For example, your start() method in BootstrapApplication could be equivalently written as:

  public start = async () => {
    await this.knex.start(knexConfig);
    await this.server.start();
  };

Note that you can't safely remove the .catch(() => { ... } in assertDatabaseConnection though, as that doesn't simply rethrow the error.

You can however simplify assertDatabaseConnection down quite a bit. You don't need the new Promise at all, because you already have a promise, so you can simplify down to just:

  private assertDatabaseConnection = async () => {
    try {
      await this.knex.raw('select 1+1 as result')
    } catch (err) {
      console.log(
        '[Fatal] Failed to establish connection to database! Exiting...',
      );
      console.log(err);
      process.exit(1);
    }
  };

In general, new Promise isn't something you need for most cases in async code, it's only really necessary for some rare cases, mainly when converting callback/event-driven code into promises.

Hello thanks, in case my try catch in other cases becomes unnecessary? would it be ideal for me to just start? (async () => { const server = new BootstrapApplication (); try { await server.start (); } catch (error) { throw error } }) (); — Ming, Oct 07 '20 at 15:59
Yes - your other try/catches are unnecessary. I've edited the question a bit to make that clearer. — Tim Perry, Oct 07 '20 at 16:20

typescript use of try catch and async await

1 Answers1