7even / vkontakte_api

Ruby-адаптер для ВКонтакте API
http://7even.github.com/vkontakte_api
MIT License
283 stars 68 forks source link

add raise for execute_errors #90

Closed moklokov closed 6 years ago

moklokov commented 7 years ago

Здравствуйте.

При вызове метода execute возвращается ответ вот такого типа:
{"response":"","execute_errors":[{"method":"groups.getMembers","error_code":15,"error_msg":"Access denied: you should be group moderator"},{"method":"execute","error_code":15,"error_msg":"Access denied: you should be group moderator"}]

Так как при разборе ответа идет проверка на присутствие поля error, то данный ответ не считается ошибкой и приложение не выбрасывает VkontakteApi::Error и пытается обработать результат.

Данная проблема уже подымалась вот тут https://github.com/7even/vkontakte_api/pull/43 Но данный пул реквест уже давно висит открытым + в нем предлагается ввести еще один тип ошибок VkontakteApi::ExecuteError.

Я предлагаю выбрасывать стандартный ексепшин VkontakteApi::Error просто в поле method отобразить цыпочку вызовов "execute -> groups.getMembers", так как поля error_code, error_msg просто дублируются.

7even commented 6 years ago

@moklokov а что если там будет 2 ошибки в разных методах? Что-нибудь вроде этого, например:

{
  "execute_errors": [
    {
      "method": "status.get",
      "error_code": 15,
      "error_msg": "Access denied: no access to call this method"
    },
    {
      "method": "groups.getMembers",
      "error_code": 125,
      "error_msg": "Invalid group id"
    },
    {
      "method": "execute",
      "error_code": 125,
      "error_msg": "Invalid group id"
    }
  ]
}

Тогда в атрибуте method ошибки будет что-нибудь типа execute -> groups.getMembers -> status.get, что неправильно, потому что execute вызывает status.get, а потом execute вызывает groups.getMembers.

Я думаю, что дополнительный класс типа VkontakteApi::ExecuteError всё-таки нужен, а в нём должна быть как информация из последнего хэша (где method: 'execute'), так и обо всех вложенных ошибках (все хэши, кроме последнего), например в виде массива объектов VkontakteApi::Error.

moklokov commented 6 years ago

Я так понимаю что данный пример придуман из головы, правильно? Я думаю что данной ситуации быть не может, так как после первой встречной ошибки код прервет свое выполнение и до второй ошибки не дойдет и вернет массив с 2 ошибок.

7even commented 6 years ago

К сожалению, это реальный пример:

VkontakteApi.configure do |vk|
  vk.log_responses = true
end

code = 'return [API.friends.get(), API.status.get(), API.photos.get()];'
vk.execute(code: code)
# {"response":[[3115,237737,345257,...],false,false],
# "execute_errors":[{"method":"status.get","error_code":15,"error_msg":"Access denied: no access to call this method"},
# {"method":"photos.get","error_code":100,"error_msg":"One of the parameters specified was missing or invalid: album_id is invalid"},
# {"method":"execute","error_code":100,"error_msg":"One of the parameters specified was missing or invalid: album_id is invalid"}]}

Конечно, пример надуманный - но важно то, что в принципе execute может возвращать больше одной ошибки за раз, и гем не должен это игнорировать.

moklokov commented 6 years ago

Да, я был неправ execute может возвращать более 2 ошибок. Тогда имеет смысл ввести дополнительный тип ошибки VkontakteApi::ExecuteError, что и было предложено вот в этом пул реквесте https://github.com/7even/vkontakte_api/pull/43, но он уже весит открытым с 2014 года, почему так? У меня в проекте описанная проблема вызова метода execute есть, и она гемом игнорируется, просто возвращая пустой ответ.

7even commented 6 years ago

Я немного доработал и смёрджил #43, можно попробовать подключить гем с гитхаба и проверить.