Search code examples
typescriptknex.js

typescript use of try catch and async await


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


Solution

  • 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.