Хочется прокидывать auth token или user info в контексте по всей цепочке, чтобы бизнес логика могла на это ориентироваться.

Перевести тесты на proper.

Версия: 249fa01d1385babf7da96aeb82d9ed006d55465d.

В спеке заявлено наличие поля с типом в виде мапы:

-type meta_invoke_service_handler() :: #{
    service  => woody:service_name(),
    function => woody:func(),
    args     => woody:args(),
    metadata => woody_context:meta()
}.

Но в реальных событиях приходит атом undefined. Я ожидал, что там будет пустая мапа.

Согласно текущим требованиям к RPC lib на стороне клиента root запрос - всегда единственный и создаётся именно woody клиентом.

Более гибкий подход: в woody клиенте одинаково работать со всеми запросами, как с запросами относящимися к дочерней ноде dapper дерева, и при этом в api предоставить функцию/метод для генерации валидного контекста root запроса, который затем и передавать в api call woody клиента. Это позволит строить dapper дерево, включающее все запросы в рамках бизнес процесса, даже если он генерируют несколько root запросов в текущем определении такого запроса в woody.

Такое изменение потребует корректировку требований RPC lib и, вероятно, обновление всех остальных реализаций woody библиотеки.

Сейчас каждый сервис форматирует эвенты по-своему, но по факту всё хотят одного. Логично было бы сделать стандартный в самом woody.

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

После обсуждений решили, что было бы хорошо порефакторить эвенты и привести их в порядок.

Сейчас в поведении woody_server_thrift_handler в handle_function() handler_opts() — это list(), а должен быть просто term().

Судя по https://github.com/rbkmoney/woody_erlang/blob/master/src/woody_event_handler.erl#L190 не умеет форматировать бизнес-исключения, из-за чего мы наблюдаем следующие сообщения в логах:

13:58:30.834 [warning] [6o4igxur3 675339606916333568 675339612524118016]unknown woody event type 'service result' with meta #{result => {payproc_ClaimNotFound},status => exception}

Ошибка

Когда на проводе мы получаем такое вот добро:

HTTP/1.1 500 Internal Server Error
server: Cowboy
date: Fri, 26 Aug 2016 11:51:56 GMT
content-length: 103
x-rbk-parent-id: 6k37eS4HH
x-rbk-span-id: CIwIGXFAAAA=:1
x-rbk-trace-id: 6k37eS4HH
x-rbk-rpc-error-thrift: application exception unknown
content-type: application/x-thrift

........Start..........GAn error occurred: {exception_not_declared_as_thrown,{'MachineFailed'}}........

В ивенте светится только следующая информация:

#{result => {transport_error,server_error},status => error}

Поддержка https (как будущее требование к RPC библиотеке).

whatever

Реализовать поддержку различных thrift протоколов в зависимости от http хэдера content-type.

Код упал с noproc, в логах

18:58:58.907 [debug]  internal error: #{error => <<"http handler terminated abnormally">>,reason => {case_clause,{exit,{noproc,{gen_server,call,[{via,gproc,{n,l,{mg_storage_test_server,<<"incorrect_event_sink_id">>}}},{get_machine,<<"incorrect_event_sink_id">>}]}}}}}

Доп копания показали, что case_clause во внутренностях вуди

{[{reason,
      {case_clause,
          {exit,
              {noproc,
                  {gen_server,call,
                      [{via,gproc,
                           {n,l,
                              {mg_storage_test_server,
                                  <<"incorrect_event_sink_id">>}}},
                       {get_machine,<<"incorrect_event_sink_id">>}]}}}}},
  {mfa,{woody_server_thrift_http_handler,handle,2}},
  {stacktrace,
      [{woody_server_thrift_handler,handle_function_catch,6,
           [{file,
                "/Users/petrkozorezov/Desktop/workspace/rbk_money/petrkozorezov/machinegun/_build/default/lib/woody/src/woody_server_thrift_handler.erl"},
            {line,198}]},
       {woody_server_thrift_handler,process,1,
           [{file,
                "/Users/petrkozorezov/Desktop/workspace/rbk_money/petrkozorezov/machinegun/_build/default/lib/woody/src/woody_server_thrift_handler.erl"},
            {line,96}]},
       {woody_server_thrift_handler,start,5,
           [{file,
                "/Users/petrkozorezov/Desktop/workspace/rbk_money/petrkozorezov/machinegun/_build/default/lib/woody/src/woody_server_thrift_handler.erl"},
            {line,69}]},
       {woody_server_thrift_http_handler,do_handle,5,
           [{file,
                "/Users/petrkozorezov/Desktop/workspace/rbk_money/petrkozorezov/machinegun/_build/default/lib/woody/src/woody_server_thrift_http_handler.erl"},
            {line,317}]},
       {cowboy_handler,handler_handle,4,
           [{file,
                "/Users/petrkozorezov/Desktop/workspace/rbk_money/petrkozorezov/machinegun/_build/default/lib/cowboy/src/cowboy_handler.erl"},
            {line,111}]},
       {cowboy_protocol,execute,4,
           [{file,
                "/Users/petrkozorezov/Desktop/workspace/rbk_money/petrkozorezov/machinegun/_build/default/lib/cowboy/src/cowboy_protocol.erl"},
            {line,442}]}]}

Да, и не хватает стектрейса ;)

Для удобства тестирования сделать супервизор для woody_server из коробки. Реальные приложения, всё также должны запускать woody_server в рамках своего!

Функция woody_server_thrift_handler:process/1 не возвращает протокол, в случае ошибки чтения thrift_protocol:read(Protocol, message_begin).

Надо:

    case MessageBegin of
        ...
        {error, Reason} ->
            {handle_protocol_error(State1, Reason), State1#state.protocol}
    end.

Не хватает возможности прокинуть параметры в event handler. Например, чтобы использовать один и тот же модуль для клиента и для сервера, при этом понимая, кто, что пишет.

Порефакторить event_handler behavior для более удобной работы с логированием в его реализации. Например, в woody_server_thrift_handler пользовательский handle_function теперь всегда возвращает WoodyContext и он тоже попадает в result поле метадаты вместе с thrift ответом, засоряя лог события.

Связана с Issue 12.

Сделать более понятный handler спек для woody_server:thrift_handler:

{
    ServiceSpec :: {
        Path,
        Service :: {ThriftHandler :: module(), ServiceThriftName :: atom()}
    },
    HandlerImpl :: module(),
    HandlerOpts :: term()
}

Возможно (в контексте Issue 12), даже, имеет смысл объединить последние 2 параметра в:

HandlerModOpts :: {HandlerImpl :: module(), HandlerOpts :: term()}

Всё в типы завернуть, разумеется.

subj

Убрать поддержку создания именно коренного запроса из woody_client, дабы унифицировать логику работы с контекстом на клиенте и сервере и избавиться от флага #root_rpc в woody_context:ctx().

Выделить среди множества полей woody meta к разным событиям, те которые относятся ко всей обработке rpс запроса (например, service, function, type) и добавлять их в метаданные во все woody event'ы (для этого можно сохранять их в woody context).

В woody_client_thrift функция format_return делает немного не то, что от неё ожидается из её названия. Функции форматирования обычно переводят один формат данных в другой, и когда они падают это значит, что что-то не так с данными. Тут же функция по смыслу обрабатывает результат (handle_result), т.к. в результат не только переформатируется, но и некоторых случаях кидается исключение. И когда видишь стек трейс в котором всё заканчивается на format_return, первая мысль, что что-то взывается внутри.

В woody_client_thrift_http_transport во многих случаях детали ошибки, полученной в процессе вызова сервера, приводятся только в woody event'е, но не возвращаются клиенту.

Например:

handle_result({error, Reason}, Context) ->
     _ = log_event(?EV_CLIENT_RECEIVE, Context, #{status => error, reason => woody_error:format_details(Reason)}),
     {error, {system, {internal, result_unexpected, <<"http request send error">>}}}

Это сделано намеренно, так как не предполагается, что клиентский код будет завязывать бизнес логику на поле error details. Полученные детали ошибки при этом доступны для интроспекции через woody event.

Тем не менее, поступило предложение (@keynslug), все-таки передавать детали ошибки клиенту.

Обдумать и обсудить.

Для случаев, когда клиент ожидает какого-то события от сервера добавить инструментами woody подписаться на такое событие. Реализовать это можно, например, с помощью long pooling'а event'ов.

Сейчас очень не хватает транзиентных ошибок (в http это 503).
Так же было бы полезно иметь bad gateway и gateway timeout.
Кроме того были мысли в cds заменить исключение KeyringLocked на 503 c коментарием (нужно придумать как коментарий к ошибкам сделать).

В результате в логах появляется вот такое:

16:32:54.541 [warning] [6oguXdgA3 684438162675597312 684438162780454912]unknown woody event type 'service result' with meta #{result => {payproc_NoLastEvent},status => exception}

Означенное событие эмиттится в woody_client_thrift:93 после получения клиентом бизнес-исключения.

Версия: b5ae9ae3abc8da470f68d2e2ca15ccc606801225.

Сейчас woody:call в ответ возвращает {{ok, V},Context}, хотя интерфейс кидающий и атом ok там не нужен. Поэтому предлагается сделать просто {V,Context} если есть значение в ответ, и {ok,Context} если значения нет.

1. Тип woody_error:details() можно сделать более гибким (например, вообще, term()) и форматированием его в binary() заниматься на сервере непосредственно при построении http ответа.
2. Сейчас (commit 992c279466e7eb1c24a9d9c6e7e8d66c597bc7e1) stacktrace неожиданной ошибки сервера (result_unexpected) прокидывается клиенту в http заголовке woody.error.reason. Корректно передавать такой stacktrace в body, а не в http заголовке. Надо задокументировать это, как общее требование к библиотеке и там же описать схему миграции (аналогично схеме миграции именования woody хэдеров). Затем поправить реализацию в woody_erlang.

К какому классу относить и как рапортовать ошибки woody extension.

подробности вот в этом комменте

• When debug is enabled (option enable_debug=true) print the request body as well (or make the option more granular, so one can choose to print only headers or headers+body).
• Make it possible to enable debug in runtime (not only on woody server startup)