Заметка о переопределнии canRequest в документации

[km256]

Недавно мы очень сильно отхватили на неаккуратном переопределении canRequest Наша ошибка заключалась в том, что в переопределённом методе мы не реализовали логику про лимит перезапросов, которая находится в оригинальном методе. В результате в случае нештатного поведения сервера в определённых условиях ручка всегда отвечала пятисотками, а клиент её бесконечно перезапрашивал.

Есть предложение добавить в документацию (https://yandex-ui.github.io/noscript/single-page/#index_md_25) небольшую заметку, что по умолчанию в этот метод зашита логика про три перезапроса и при переопределении метода, это стоит учитывать?

Либо, чтобы обезопасить пользователей от таких вот казусов, вообще сделать canRequest исключительно публичным, а проверку лимита перезапросов делать в отдельном приватном методе? Что скажете?

[alexeyten]

Я вот тоже подумал про приватный метод.

[vitkarpov]

нештатного поведения сервера в определённых условиях ручка всегда отвечала пятисотками

Все так, но, строго говоря, ручка отвечала 200, просто в data ответа было null (бекенд навернулся) — такой запрос считается FAILED и должен быть перезапрошен.

[vitkarpov]

/cc @chestozo @Katochimoto

Коллеги, есть предложение логику, которая сейчас в canRequest про лимит перезапросов, вынести в приватный метод, а публичный метод по дефолту будет всегда говорить true и будет доступным для переопределения.

Что скажете?

[chestozo]

Мне кажется надо поставить защиту от бесконечных запросов (типа retries не больше 100, к примеру) + в консоль варнинг писать. Мне кажется тут 2 метода добавят сумятицы. Ну т.е. кейс ясен, но тут явный косяк, нужно было повнимательнее посмотреть в оригинальный метод, имхо.

[vitkarpov]

Мне кажется тут 2 метода добавят сумятицы.

Это не два метода, публичный метод по-прежнему один. Просто логика, которая сейчас в нем находится должна быть в приватном методе, который ты, по идее, не можешь просто так переписать. А публичный по прежнему меняй сколько угодно.

Таким образом, мы защитимся от «случайного» переписывания.

[vitkarpov]

Мне кажется надо поставить защиту от бесконечных запросов

Логика с 3 запросами кажется ок. Она должна быть прибита гвоздями. пользовательский canRequest это скорее про «невозможность запросить модель вовсе, при определенных условиях», а про перезапросы пользователь не должен думать.

[chestozo]

Ну вот моё имхо - предлагаемое будет улучшением 50/50. Вы ведь можете решить проблему сейчас переопределив canRequest иначе? Если да - предлагаю ничего не делать )

[vitkarpov]

Вы ведь можете решить проблему сейчас переопределив canRequest иначе

Да, разумеется. Мы все зафиксили. Здесь речь идет о том, чтобы убрать возможность выстрелить себе в ногу. Кажется, что если такая возможность есть без усложнения публичного АПИ — это добро.

[chestozo]

Мне кажется в данном случае кейс классифицирован как "выстрелить в ногу" с натяжкой. Давайте ещё мнения соберём.

[vitkarpov]

@Katochimoto @alexeyten @shirokoff что думаете?

[alexeyten]

но тут явный косяк, нужно было повнимательнее посмотреть в оригинальный метод

Это какая-то кривая отмазка. Я вот был уверен, что логика про лимит перезапросов живёт где-то внутри и её нельзя так легко и просто потереть.

Я за разделение на _canRequest который делает стандартные проверки и вызывает canRequest который по умочанию всегда возвращает true и его можно переопределять.

[chestozo]

Ну как бы переопределяешь метод - загляни, чего там внутри, разве нет?

[alexeyten]

С чего бы. Для этого документация есть. И в ней нигде явно не написано большими красными буквами, что нужно ещё и количество ретраев в переопределении проверять. Это значит, что либо надо править документацию (изначальное предложение этого тикета), либо править код.

Я за правку кода, потому что не вижу ни одной причины почему бы кто-то хотел убрать проверку лимитов при переопределении метода. А если кто-то вдруг захочет, то у него всегда есть возможность поднять лимит (RETRY_LIMIT) с 3 до бесконечности.

[chestozo]

С чего бы. Для этого документация есть.

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

Я в общем не против, но мне это кажется пока больше опциональным.

[vitkarpov]

Мир не идеален, к сож.

Мы движемся в идеальный :) Учитывая, что это никак не затрагивает текущее публичное АПИ, то кажется, годной темой.

Давай сделаю пул и посмотрим еще раз.

[vitkarpov]

Заметку добавлять не будем.