FoalTS / foal

Full-featured Node.js framework, with no complexity. 🚀 Simple and easy to use, TypeScript-based and well-documented.
https://foalts.org/
MIT License
1.88k stars 137 forks source link

Closing connection in shell script #645

Closed anonimusprogramus closed 4 years ago

anonimusprogramus commented 4 years ago

In this create-user script, we have conditional checking:

await createConnection();

/* ... redacted */

if (await isCommon(args.password)) {
  console.log('This password is too common. Please choose another one.');
  return;
}

If the condition is met (too common password), then exit the script.

Question:

Did that return; statement automatically close the connection, so we don't need await getConnection().close(); ?

Thank you

LoicPoullain commented 4 years ago

Actually it does not, which is bad.

We should have something like this instead (with a try/catch/finally):

// imports (modified)

export async function main(args: { email: string, password: string }) {
  const connection = await createConnection();
  try {
    const user = new User();
    user.email = args.email;

    if (await isCommon(args.password)) {
      console.log('This password is too common. Please choose another one.');
      return;
    }
    await user.setPassword(args.password);

    console.log(
      await getManager().save(user)
    );
  } catch (error) {
    console.log(error.message);
  } finally {
    await connection.close();
  }
}

The same applies for the create-todo script (for the first and second tutorials) and the script described here: https://foalts.gitbook.io/docs/topic-guides/authentication-and-access-control/groups-and-permissions#creating-groups-with-a-shell-script-cli

If you feel like to open a PR to surround these codes with a try/catch/finally, feel free to do it. I'll be happy to review. No need to add new comments or add explanation, just the code can be updated. The files to update would be:

anonimusprogramus commented 4 years ago

I'm gonna take this good opportunity, one by one. Thank you.