Корректно ли пишу код NodeJS?

Question

[Андрей Рудаков]

Ранее не писал запросы к базе Postgresql на NodeJS.
Мой код выглядит так, но мне кажется, что есть более корректный способ:

app.post('/register', async (req, res) => {
    let body = req.body
    let response = {status:'', title:'', text:''}
    await checkUniqueEmail(body.email).then(async function (result) {
        if (result === 0) {
            response.status = 'success'
        } else {
            response.status = 'error'
            response.title = 'E-Mail адрес занят'
            response.text = 'Используйте другой'
        }
    })
    res.json(response)
});

async function checkUniqueEmail(email) {
    return new Promise(function(resolve, reject) {
        pool.query(`SELECT * FROM users WHERE 'email' = $1`, [email], (err, result) => {
            if (err) {
                console.log(err)
            } else {
                resolve(result.rowCount)
            }
        })
    })
}

В данном случае идёт проверка на уникальность E-Mail адреса.

Comments

[WbICHA]

мне кажется что есть более корректный способ

Не кажется. Тут всё, по сути, надо переписывать.
Но вначале следует разобраться с асинхронностью, потому что сейчас понимание если и есть, то на зачаточном уровне.

[Андрей Рудаков]

WbICHA, Спасибо.

app.post('/register', async (req, res) => {
    let body = req.body
    let response = {status:'', title:'', text:''}
    checkUniqueEmail(body.email, function (err, result) {
        if (result.rowCount === 0) {
            response.status = 'success'
        } else {
            response.status = 'error'
            response.title = 'E-Mail адрес занят'
            response.text = 'Используйте другой'
        }
        res.json(response)
    })
});
function checkUniqueEmail (email, callback) {
    pool.query(`SELECT * FROM users WHERE 'email' = $1`, [email], (err, result) => {
        callback(err, result)
    })
}

Переписал, так лучше?

[WbICHA]

Андрей Рудаков, относительно. Оно теперь хотя бы корректно работать должно.

[Андрей Рудаков]

WbICHA, Прошу прощение за доставучесть, может этот вариант вернее?)

app.post('/register', async (req, res) => {
    let body = req.body
    let response = {status:'', title:'', text:''}
    if (await checkUniqueEmail(body.email)) {
        response.status = 'success'
    } else {
        response.status = 'error'
        response.title = 'E-Mail адрес занят'
        response.text = 'Используйте другой'
    }
    res.json(response)
});
function checkUniqueEmail (email) {
    return pool.query(`SELECT * FROM users WHERE 'email' = $1`, [email]).then(res => {
        if (res.rowCount !== 0) {
            return Promise.resolve(false)
        } else {
            return Promise.resolve(true)
        }
    }).catch(err => Promise.reject(err))
}

[WbICHA]

Андрей Рудаков, хорошо, так и быть, хотя бы упорство имеется.

app.post('/register', async (req, res) => {
    try {
      const { body } = req;
      const isUniqueEmail = await checkUniqueEmail(body.email);

      if(!isUniqueEmail) {
        res.json({
          status: 'error',
          title: 'E-Mail адрес занят',
          text: 'Используйте другой',
        });
        return;
      }

      res.json({
        status: 'success',
      });
    } catch (err) {
      // ...
    }
  });

async function checkUniqueEmail(email) {
    // Понятия не имею что это за функция, поэтому исхожу из того, что вижу в твоём коде
    return pool.query(`SELECT * FROM users WHERE 'email' = $1`, [email])
      .then(res => res.rowCount === 0);
  }

[Андрей Рудаков]

WbICHA, Огромное спасибо за помощь :)

[Андрей Рудаков]

WbICHA, Это снова Я, Если уже надоел, можете не отвечать)
Пришлось поменять код исходя из pool.connect рекомендаций и прежний код не вязался.

app.post('/register', async (req, res) => {
    try {
        const { body } = req
        await checkUniqueEmail(body.email, function (isUniqueEmail) {
            if (!isUniqueEmail) {
                res.json({
                    status: 'error',
                    title: 'E-Mail адрес занят',
                    text: 'Используйте другой',
                })
                return
            }
            res.json({
                status: 'success',
            })
        })
    } catch (err) {
        console.error(err)
    }
});

function checkUniqueEmail (email, callback) {
    pool.connect((err, client, done) => {
        if (err) throw err
        client.query('SELECT * FROM users WHERE email = $1', [email], (err, res) => {
            done()
            if (err) {
                console.log(err)
            } else {
                return callback(res.rowCount === 0)
            }
        })
    })
}

В правильном направлении?

[WbICHA]

Андрей Рудаков, checkUniqueEmail реализована чуть более, чем неверно. Изучай асинхронность.

[Андрей Рудаков]

WbICHA, Но тем не менее, это вроде как работает. Принцип написания checkUniqueEmail нашёл на просторах интернета

[WbICHA]

Андрей Рудаков, работает? Да, безусловно, я бы немного удивился, если бы не работало в идеальных условиях.
А теперь представим: БАЦ и на любом из "этапов" вылетает рандомная ошибка. Вот ты и поиграл.

[Андрей Рудаков]

WbICHA, Я просто ещё не дошёл до тех мест, где написано console.log(err). Понято что их нужно обработать. Вы об этом?

[WbICHA]

Андрей Рудаков, да.

Answer 1

[Philipp]

Посмотри knex, joi.

// Для начала разбиваем код на отдельные обработчики и храним все раздельно
// например в signup.ts
import { ERR_USER_EXISTS, ERR_USER_CREATION_FAILED } from './utils/error-codes.ts'
import { planAction, USER_CREATED } from './event-bus/rabbit.ts'
import { validateSignUpData } from './validator/signup.ts'
import { userModel } from './model/user.ts'

// собственно обработчик
export const signup = async (req, res) => {
    const { body } = req // намного удобнее использовать destructuring
    const { email, password } = body  // и const позволяет контролировать неизменность

    try {

        validateSignUpData({email, password}) // зависит от бизнес-логики

        const isUserExists = await userModel.isUserExists(email)
        if (isUserExists) {
            res.status = 400
            res.json({
                message: 'User exists',
                code: ERR_USER_EXISTS // вместо написания сообщений об ошибках
                                      // принято возвращать код ошибки
                                      // это позволяет адаптивную локализацию на фронте
                                      // этот код ошибки отличается от HTTP Status Code 
                                      // https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
                                      
            })
            return
        }

        // попытаемся создать пользователя
        const userInfo = await userModel.createUser({email, password})
        if (!userInfo) {
            // в зависимости от того, как реализован createUser, он может выбрасывать исключение
            // либо возвращать пустоту
            res.status = 400
            res.json({
                message: 'User creation failed',
                code: ERR_USER_CREATION_FAILED
            })
            return
        }

        // запланируем отсылку события, обычно этот код должен быть реализован асинхронным
        // и не блокировать основной поток событий
        planAction({
            event: USER_CREATED,
            playload: userInfo
        })

        // отправим успешный ответ на фронт
        res.status = 200
        res.json(userInfo)

    } catch (e) {
        // любая ошибка, где бы она не возникла, должна быть обработана
        // в целом обычно применяется более сложная система с набором специфичных исключений 
        // и общим обработчиком ошибок
        res.status = 501
        res.json({
            message: e.message,
            code: e.code || 0
        })
    }
}


// далее в app.ts
// затем мы просто привязываем их
import { signup } from './signup.ts'
app.post('signup', signup)

Comments

[Андрей Рудаков]

Спасибо. Примерно так уже реализовал)