[bug] vk.api отправляет параметры, равные undefined

[zardoy]

Привет!
Если коротко, то вызов любого API с параметром, у которого явно указано значение undefined приводит к тому, что он передается ВК. Ниже привел пример.

What did you do?

await ctx.send("it'll fail", {
    keyboard: undefined
});

ctx – контекст события message_new

What did you expect to happen?

Параметры со значением undefined не должны передаваться, ровно так же, как если бы они были опущены.

What was the actual result?

VK-IO переводит undefined в строку и отдает это серверу.

Versions

package	version
vk-io	4.0.1
node	12.17.0
TypeScript	4.0.2
yarn	1.22.4

[mulfyx]

Нет, постой, это не так работает немножко. Тебе объяснить?

[zardoy]

Ну давай, что немножко не так?

[mulfyx]

То, что параметры отправляются с помощью URLSearchParams

const params = {
   message: 'hello',
   keyboard: undefined
};

console.log(new URLSearchParams(params));
// Выведет: URLSearchParams { 'message' => 'hello', 'keyboard' => 'undefined' }

[zardoy]

@isinkin Да, но я в PR уже написал, что это именно баг, тк TypeScript вообще без разницы параметр не указан или указан со значением undefined, а вот в runtime это уже приводит к ошибки API. Это прям подводный камень какой-то.

[tuogeniy]

@zardoy почему это баг?

const obj = {
  key: undefined,
}

const properties = Object.getOwnPropertyNames(obj)

console.log(properties)

Свойство объявлено, его тип - undefined и при его преобразовании в строку получается строка 'undefined'.
То что у ts что-то там не так работает уже относится к ts, рантайм у нас на v8, как правило, и он поддерживает только javascript.

[zardoy]

const obj = {
  key: undefined,
}

const properties = Object.getOwnPropertyNames(obj)

console.log(properties)

Все верно, но только зачем это преобразование? Ведь если мы хотим явно передать строку с таким контентом, почему явно не передать строку в качестве параметра?

vk.api.messages.send({
    message: "undefined" // - так ок
});

Возможно ты прав, что это не баг, а просто подводный камень из-за того, что ts не выкидывает ошибку компиляции при явном указании undefined. Однако моё предложение в PR заключается в том, что бы поведение когда мы параметр опускаем полностью и указываем undefined было одинаковым, тк не всегда

[zardoy]

Сори, что закрыл, сейчас дополню.

[tuogeniy]

@zardoy я понимаю о чем ты.

Наверняка ты пишешь что-то вроде

let someVariable;

const query = {
  key: someVariable,
}

send(query)

Я считаю эта проблема на стороне пользователя, писать нужно по-другому, а этим pr-ом ты ломаешь обратную совместимость.

[zardoy]

Я считаю эта проблема на стороне пользователя, писать нужно по-другому, а этим pr-ом ты ломаешь обратную совместимость.

Согласен.

Просто вот смотри где собака зарыта (более наглядный пример):

• я не мог так прописать

• я вот так уже могу

ts ругаться ну будет, хотя запрос по факту будет таким же. Просто подводный камень для пользователя

[tuogeniy]

@zardoy так ты ничего не пиши если не хочешь передавать этот параметр, будет работать так как ты хочешь

vk.api.messages.send()

[negezor]

Тут скорее несовпадение поведений, при JSON.stringify() будет упущен undefined. В то время когда URLSearchParams приводит всё к строке. Это не будет ломанием обратной совместимости, так как проверяется именно undefined, а не "undefined".

Но теперь мы потеряем интересные сообщения в чате в виде undefined :)

[tuogeniy]

@negezor про такие сообщения я и имел ввиду, не исключено что кто-то использует тип undefined когда хочет напечатать такой текст)

upd: ну, а вместо undefined будет вылетать exception в некоторых случаях, например когда сообщение пустое.

[mulfyx]

А зачем вы вообще указываете в параметрах undefined?

[zardoy]

Явное указание — упрощенный пример, лишь для понимания. В реальности же это значение может приходить из функции, при слиянии объектов, и много откуда еще. Вот пример из PR:

await ctx.send("Hey` there!", {
    keyboard: await getKeyboardToSend() //undefined | KeyboardBuilder - соответствие типов, но ошибка ругается API
});

Здесь использую как резервной тип, который бы означал то, что не нужно показывать клавиатуру

[tuogeniy]

То что URLSearchParams собирает undefined наверное более гибко и правильно, но менее удобно.

Я плохо знаю vkapi и сурсы этой либы и не могу сказать есть ли проблема, но если проблема разных поведений в одной библиотеке действительно есть, то это надо решать.

Полагаю, что можно передать напрямую body запроса и оно преобразуется в JSON с помощью JSON.stringify, который усекает undefined в отличии от URLSearchParams, это путает пользователей.

[zardoy]

То что URLSearchParams собирает undefined наверное более гибко и правильно, но менее удобно.

@talentumtuum не совсем согласен, могу даже пример из другой весьма крупной либы привести.
Есть такая библиотека prisma (через неё можно делать запросы к бд). И вот один из их примеров:

const hisNotes = await prisma.userNote.findMany({
    where: {
        userName: "Dmitriy" // вернет записки только дмитрия
    }
});
const allNotes = await prisma.userNote.findMany({
    where: {
        userName: undefined // не вернет записки пользователя "undefined"
    }
});
// это равносильно
allNotes = await prisma.userNote.findMany({ });

[zardoy]

upd: ну, а вместо undefined будет вылетать exception в некоторых случаях, например когда сообщение пустое.

согласен, только вот представь если тебе бот на любую команду будет спамить "undefined". Лучше уж исключение чем silent бессмыслица xd

upd: может хотя бы в логах тогда увидят, да пофиксят

[negezor]

Всё же я думаю ошибка лучше, чем проблемный пост. В таких инструментах как Sentry сложно отследить то что не является ошибкой. Мажор 4 версии был нацелен на улучшенное взамодействие с библиотекой и явный undefined не является им.

Это так же исправляет неочевидное поведение между URLSearamParams и JSON.stringify(), исправлено в 1026d33, опубликовано в 4.0.2