spec icon indicating copy to clipboard operation
spec copied to clipboard

feat: request/response support in both operation and message objects

Open smarek opened this issue 3 years ago • 20 comments

Title: "Request/Response paradigm support in both operation and message objects" Related issue(s): #55 #94 #558 Champion: @smarek


Since nobody yet took the opportunity, i'm gonna try to champion this Feature RFC.


Explanation of Problem & Solution:
Very common type of operation is currently missing in specification per discussion in referenced (related) issues. Operation that results in direct or indirect response to the caller. The most basic example is publishing a message, where the result of publish operation should be known as soon as possible to the caller. Eg. invoking operation on given channel will return the specific message (or oneOf messages) right away to the caller. The complicated example is correlation of the request and response, where publish/subscribe operation with given message will result in the specific message to be delivered back to the caller via different channel.

Solution has two parts:

  1. Allowing referencing single or multiple messages as a result to operation (indifferent to whether the operation is server or client initiated)
  2. Allowing hinting single or multiple messages that can be expected by other party to be received as a result to using specific message in any kind of operation

Which allows for "Acknowledge" messages, "HTTP Request/Response" correlation and foremost to better describe (and for api document reader understand) the situations where one might receive specific message


Illustrative examples:

I'll limit myself to two examples, as various are already discussed in related (referenced) issues.

  1. HTTP Message Binding - Request and Response - HTTP standard expects the caller to consume the response headers and possibly a response payload, if given HTTP verb allows it. - This feature would allow to specify response headers and json body that would represent the result of HTTP GET, POST, PUT or other http operation
  2. WebSocket protocol binding / (SignalR, RPC) - SignalR interface specifies two-way methods in rpc-like manner, where client or server can invoke operations (interface methods/functions) on each other, where each operation can take arguments and can produce any kind of response (be it scalar value or complex objects), the transport is JSON over WebSocket - This feature allows the SignalR protocol to correctly define the messages that client/server can receive as a result of invoking operation on the other party OR invoking operation with specific message type

Also mentioned should be usage of current x-response extension https://www.asyncapi.com/blog/websocket-part2#describe-responses---specification-extensions


Identification of potential concerns, challenges, and drawbacks:

  1. Current spec edit proposal allows for infinite chaining of the responses, which if not implemented asynchronously, would result in terrible call-stack-overflows - I actually think it is desired and it should be concern of user/developer whether the response message chaining is used correctly or not
  2. Allowing specifying different kind of messages on operation as well as on message might create ambiguous situations, where the consumer must account both for messages that can be produced on operation and messages that are related to "request message" - This is in my opinion necessary to support as wide spectrum of bindings/protocols as possible, some protocols produce different kind of messages on single channel item operation (eg. getNextOfAnyType kind of operations) and some protocols use the invoke/getResult paradigm where the response message is always directly linked to the invoked operation and "request message" used in such
  3. My last concern is that making both of these result fields OPTIONAL is not long-lasting solution and will not drive the community to use the option to make their API documentations more strong-typed, and I'm honestly not sure whether there is anything we can do about it

smarek avatar Jul 27 '21 20:07 smarek

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Jul 27 '21 20:07 sonarcloud[bot]

@fmvilas @derberg could you please, if applicable, add RFC label to this PR?

smarek avatar Aug 09 '21 10:08 smarek

@fmvilas from my point of view this is Stage 1: Proposal. Nothing is missing for Stage 1 I think

derberg avatar Aug 09 '21 11:08 derberg

Yup. Thanks for noticing, @smarek.

fmvilas avatar Aug 09 '21 14:08 fmvilas

Proposal shared with the wider community:

derberg avatar Aug 11 '21 12:08 derberg

Thanks for @smarek for the interesting proposal.

Reading through I had two thoughts:

  • As this is AsyncAPI (and there exists OpenAPI, which lives to represent synchronous HTTP calls), perhaps it's not in scope to model synchronous HTTP request/reply scenarios. Maybe this plays into @fmvilas vision of linking together various APIs into an uber API, rather than having AsyncAPI be able to handle it.
  • That said, the larger vision of modeling async causality amongst n actor/endpoints is intriguing to me. @clemensv mentions this in https://github.com/asyncapi/spec/issues/601. @smarek is right that knowing what to expect in response to a publish is important. But for many enterprises it's also important to understand the choreography of multiple endpoints.

In that way, limiting this functionality only to request reply amongst two consumers is perhaps not generic enough. It might be worth exploring how to handle choreography within the spec that handles "defining multi-directional conversations abstractly: Not just request/reply, but also scatter/gather patterns, saga patterns, etc.", as @clemensv puts it.

jessemenning avatar Aug 11 '21 12:08 jessemenning

@jmenning-solace thank you for discussion as well

  • your first point is really described in initial comment, section "Illustrative examples", point 2. Where WebSocket is full-duplex, asynchronous, protocol, and so is SignalR over WebSocket. However this particular async api/application protocol (and definitely not only one that does that) supports typed interfaces where response to given request (both from client and server side) is expected and validated. Which is also the original reason I became interested in getting this topic solved in spec, current inability to link the endpoint reaction to the request. In low-level there is "correlation id" that links the request and response, but from the consumer/integrator point-of-view it is necessary to define the meaningful relation between the two messages, if the protocol supports/expects it. But since the operation/message level reaction (response) is not necessary part of any async api, both options are specified as OPTIONAL, so you can describe such asynchronous protocol with AsyncAPI but you're not forced to do so
  • i did not know the mentioned https://github.com/asyncapi/spec/issues/601 and it is indeed tricky to describe, but i'm not sure if the choreography can be described well using declarative language, such as asyncapi spec, since i'd expect the choreography to be based in some kind of turing/state-full machine and that is imho topic/challenge completely different from describing API/RPC interfaces. Regardless of what I just said, i don't think we should abandon my PR since many APIs can be sufficiently descibed with request/response causality, and the higher abstraction, enterprise point-of-view, can be added later on top of this or side-by-side

smarek avatar Aug 11 '21 16:08 smarek

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Oct 19 '21 11:10 sonarcloud[bot]

@smarek Could you please take a look at my comment? I'd like to hear from you and see how we can evolve the proposed model in order to support your requirements. As this proposal introduces breaking changes, they should be part of AsyncAPI v3.0.0. Thank you!

olamiral avatar Nov 22 '21 12:11 olamiral

To me, the request-response information should not interact with the publish nor subscribe information and thus, the request-response details should not have any interaction with the pub-sub oriented sections. I imagine something like this:

channels:
  basic:
    exchanges:
       fetchThing:
         receive:
           # ...
         respond:
           # ...
    publish:
      # ...

That is to say:

Upon channel basic, I participate the following exchanges, including the fetchThing. During the fetchThing exchange, I will receive message x and I will respond with message y.

This introduces a new, optional field of the channel object, which should (without deep understanding of the spec) maintain backwards compatibility, allowing this to slot into v2.x.

Some semantic considerations:

  1. I don't personally love the verb-oriented labels of receive/respond, however this remains consistent with the existing, verb-oriented publish and subscribe.
  2. The exchanges label a bit vague and risks becoming ambiguous when considering future pattern implementations; an "exchange" does not necessarily involve exactly two messages. (For example, a tcp handshake is a 3-message exchange.) A more specific label might be appropriate (but alternatives in my head -- such as requestResponses -- just seem too awkward).
  3. Whereas receive/respond is consistent with publish/subscribe, they occur at a different nesting level. This is not ideal, but may be necessary in order to appropriately group the information. Perhaps this lends itself to a future change (v3 and beyond) that places the exchange patten directly under the channel and then describes the messages following that pattern. (ex: channels.basic.pubSub.publish...). Like point 2, this speaks to future support of other, well-established messaging patterns.

crussell52 avatar Nov 22 '21 21:11 crussell52

I'd like to suggest another way of documenting request-reply messages:

Instead of Documenting them on the request message (or seperately on the channel) moving the Documentation to the replies can have some benefits.

channels:
  requestData:
    publish:
      message:
        $ref: '#/components/messages/requestSpecialData'
  returnChannel:
    subscribe:
      message:
        $ref: '#/components/messages/specialDataResponse'
        trigger:
          on: message
          channel: requestData
          message:
            $ref: '#/components/messages/requestSpecialData'

The samantic of this would roughly be: If a requestSpecialData message appears in channel requestData then a specialDataResponse message can be received later on the returnChannel.

benefits:

  • Triggers can be extended later to allow for timers, conditions, channel subsciption events, environment events, etc. without breaking compatibility and without introducing a new concept.
  • Triggers naturally allow to specify multiple responses
  • One only needs knowledge about what can trigger a message to correctly specify all message triggers
  • Allows other applications to extend existing request-response patterns by triggering new messages on the same condition
  • Allows specifying request-response chains across different channels, even allows for responses to channels with a variable

Disadvantages:

  • Finding replies to responses in the spec becomes harder and needs tool support (this issue is less severe if the question that is more often asked is "what do i need to do in order to get this specific message?" as triggers would answer that directly)
  • Triggers can become very complex (but we could start with defining only very simple triggers)
  • Needs more work to specify the exact semantics of triggers

I don't have a qood answer for where to put these triggers. In the current spec they should be as close as possible to the triggered messages, because we only have the two operations publish and subscribe per channel that need to describe all messages on that channel. If a channel can have any number of operations that can be specified as send or receive operations, then triggers should be specified on these operations (then we can also have triggers for when en operation fails with an error). If multiple operations per channel are allowed, then messages that have the same purpose can be grouped into a single operation that can be used to document these messages. Operations would then be logical operations that can be performed on a channel, rather than "physical" operations the channel itself allows (publish/subscribe or send/receive) like they are now. (The proposal in #618 already allows multiple operations per channel.)

I believe that triggers are a better fit for async messaging based apis because they can capture the event based or reactive behaviour of such apis. They are also more flexible then a specific solution to document request-reply message pairs and can be extended in the future.

buehlefs avatar Jan 12 '22 11:01 buehlefs

Would this work well if we wanted to describe a JSON RPC 2.0 Api ?

Here's an example request:

--> {"jsonrpc": "2.0", "method": "subtract", "params": [42, 23], "id": 1}
<-- {"jsonrpc": "2.0", "result": 19, "id": 1}

And here's an error example:

--> {"jsonrpc": "2.0", "method": 1, "params": "bar"}
<-- {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}, "id": null}

natcl avatar Mar 11 '22 14:03 natcl

Another thing to consider is that we have some subscribers that ignore messages if there is not some kind of "reply to" address provided. e.g. if I have send a message to user.free_account.signup but I don't provide a temporary topic that it can reply to, then it will just ignore my message without creating a user.

That being said, I'd like a way to specify that the subscriber will not perform any action if no reply address is given (e.g. replyTopicRequired: true).

samueleaton avatar Mar 28 '22 03:03 samueleaton

Would this work well if we wanted to describe a JSON RPC 2.0 Api ?

Here's an example request:

--> {"jsonrpc": "2.0", "method": "subtract", "params": [42, 23], "id": 1}
<-- {"jsonrpc": "2.0", "result": 19, "id": 1}

And here's an error example:

--> {"jsonrpc": "2.0", "method": 1, "params": "bar"}
<-- {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}, "id": null}

Yes, however missing "id" param in error reply is problematic, since the reply cannot be easily linked with the request (which I'd expect in async reply, and which is understandably not required when response is provided synchronously, eg. the response is expected before another request occurs)

smarek avatar Apr 26 '22 07:04 smarek

Another thing to consider is that we have some subscribers that ignore messages if there is not some kind of "reply to" address provided. e.g. if I have send a message to user.free_account.signup but I don't provide a temporary topic that it can reply to, then it will just ignore my message without creating a user.

That being said, I'd like a way to specify that the subscriber will not perform any action if no reply address is given (e.g. replyTopicRequired: true).

I agree, but that should be in my opinion part of different RFC, since it describes topic/address related flow, not just "what kind of response can subscriber expect for given request/message"

smarek avatar Apr 26 '22 07:04 smarek

@buehlefs thank you, i don't think i will adopt your proposal in this RFC, since triggers are complex topic on its own. I'm afraid that documenting which message can be passed on which channel based on which condition/trigger is separate discussion. In most cases i'd say the response to request is expected on the same (or single different, downstream) channel/topic. In terms of your example, if we have one publish/request channel and one subscribe/response channel it'd still be meaningful to identify response messages as part of requestData::publish::message along with additional identification of channel/topic where response/answer is provided. Here I'd extend on @samueleaton comment, we should probably allow documenting that "response is not mandatory" (so far my proposal allows documenting only success/error responses, but specific api design can be that no response will/might be provided at all)

That being said how about responseBehavior

channels:
  basic:
    message:
      oneOf:
        - $ref: '#/components/messages/ping'
    responseBehavior: 'mandatory' # possible values (the list can be expanded) 'mandatory', 'optional', 'success-only', 'error-only', 'no-response'
    response:
      oneOf:
        - $ref: '#/components/messages/unauthorized'
components:
  messages:
    ping:
      payload:
        $ref: '#/components/schemas/ping'
      response:
        $ref: '#/components/messages/pong'

This way the reaction (response) of the other party could be expected without going much into detail of business-logic of the application

smarek avatar Apr 26 '22 07:04 smarek

And at last, as reaction to https://github.com/asyncapi/spec/issues/94#issuecomment-1109424017 by @derberg and other mentions of this topic, seems like the general community consensus is that this RFC contains breaking changes and should not be based against 2.x but 3.x version of AsyncAPI, where others and more complex topics are currently being addressed/discussed. I'm not able nor willing to join all the discussions.

So if my understanding is correct, please close this PR and address the request/response needs in other way (more compliant with all the other changes designed in 3.x), i will not champion this topic in 3.x version.

If it's not (meaning request/reply should be part of 2.x), I vote to not complicate the topic more here and provide basic support of request/reply as already presented in this PR, for something this trivial, the discussion is taking too long and i'm losing interest.

References:

  • https://github.com/asyncapi/spec/pull/594#discussion_r736527639 - my response to 2.x/3.x and taxonomy
  • https://github.com/asyncapi/spec/pull/594#issuecomment-975949498 - complete change of PoV introducing breaking changes, which i wanted to avoid in the first place
  • https://github.com/asyncapi/spec/pull/594#issuecomment-1010946193 - same, breaking changes and trigger/conditions discussion, which should be addressed elsewhere
  • https://github.com/asyncapi/spec/issues/618#issuecomment-974603269 and whole #618 - again, trying to solve bigger problems instead of adding single, non-breaking, feature

Please let me know, and if appropriate, you can directly close this PR

smarek avatar Apr 26 '22 08:04 smarek

First @smarek great job so for and i am totally with your. There is a need to describe the response well! Describe it unrelated would make the schema very confusing, as well if it is large (as i face it often). At the moment i solve this issue with naming conventions of the '#/components/messages/*' But this is not very helpful when talking about code generator.

A little about my use case

I use messaging solution like JMS, MQTT, JCSMP, .... The you mostly have to header fields to identify the response. Options A: Well defined response topic + "correlationId". There you have a well defined response topic and use as "correlationId" to match the response to the question. Options B: Per process individual inbox aka. "replyTopic" + "correlationId". It works mostly the same as Option A. Options C: Temporary reply topic for a individual response. This is mostly preferred in a request -> multi response scenario.

Where is see problems

Where i see problems with your suggestion, there there is no possibility to define:

  • message traits
  • well defined reply topics

I dont think "responseBehavior" is an good idea:

  • Why there is a need for the option "no-response". Simply not define "response"
  • The options 'success-only', 'error-only' are error prone. I dont think it is a good idea to only response either success or error. How should the requestor act if there is no response. No answer normally means in a request reply pattern that the messages was not transmitted, the opponent is down or unable to process the message.
  • May opinion is: always answer and do it always with the same schema. Everything else will make you trouble with fixed typed programming languages (like java).

Here my suggestion:

An attribute: "channels[].subscribe.responseChannel" pointing to a message channel, can be the same but dont have to. An attribute "channels[].response" with same structure as "subscribe"/"publish" describing the response message

Example for "Options A"

channels:
  tms/monitoring/monalesy/p/v1/serviceDesc/request:
    subscribe:
      traits:
      - $ref: '#/components/operationTraits/solace'
      message:
        $ref: '#/components/messages/ping'
      responseChannel: 'tms/monitoring/monalesy/p/v1/serviceDesc/inbox/{correlationId}'
  tms/monitoring/monalesy/p/v1/serviceDesc/inbox/{correlationId}:
    description: 
      The reply topic needs to start with api name "tms/monitoring/monalesy/p/v1/serviceDesc" to not colide with other application.
      The reply topic needs to match a schema will not be blocked by ACL "tms/monitoring/monalesy/p/v1/serviceDesc/inbox/*".
    parameters:
      correlationId:
        $ref: '#/components/schemas/correlationId'
    response:
      traits:
      - $ref: '#/components/operationTraits/solace'
      message:
        $ref: '#/components/messages/pong'
components:
  messages:
    ping:
      traits:
      - $ref: '#/components/messageTraits/requestHeader'
      payload:
        $ref: '#/components/schemas/ping'
    pong:
      traits:
      - $ref: '#/components/messageTraits/responseHeader'
      payload:
        $ref: '#/components/schemas/pong'
  messageTraits:
    requestHeader:
      headers:
        type: object
        required:
        - correlationId
        properties:
          correlationId:
            $ref: '#/components/schemas/correlationId'
    responseHeader:
      headers:
        type: object
        properties:
          messageIndex:
            type: int
            minimum: 0
          messageCount:
            type: int
            minimum: 1

Example for "Options B"

channels:
  tms/monitoring/monalesy/p/v1/serviceDesc/request:
    subscribe:
      traits:
      - $ref: '#/components/operationTraits/solace'
      message:
        $ref: '#/components/messages/ping'
      responseChannel: 'tms/monitoring/monalesy/p/v1/serviceDesc/request'
    response:
      traits:
      - $ref: '#/components/operationTraits/solace'
      message:
        $ref: '#/components/messages/pong'
components:
  messages:
    ping:
      traits:
      - $ref: '#/components/messageTraits/requestHeader'
      payload:
        $ref: '#/components/schemas/ping'
    pong:
      traits:
      - $ref: '#/components/messageTraits/responseHeader'
      payload:
        $ref: '#/components/schemas/pong'
  messageTraits:
    requestHeader:
      headers:
        type: object
        required:
        - correlationId
        - replyTo
        properties:
          correlationId:
            type: string
            format: uuid
          replyTo:
            type: string
            examples:
            - tms/monitoring/monalesy/p/v1/serviceDesc/inbox/your-host-name/your-process-uuid
    responseHeader:
      headers:
        type: object
        required:
        - correlationId
        properties:
          correlationId:
            type: string
            format: uuid
          messageIndex:
            type: int
            minimum: 0
          messageCount:
            type: int
            minimum: 1

GreenRover avatar Apr 26 '22 09:04 GreenRover

super important update to folks interested in this topic.

we will host a dedicated discussion next Monday -> https://github.com/asyncapi/community/issues/352

derberg avatar May 10 '22 13:05 derberg

This pull request has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

github-actions[bot] avatar Sep 08 '22 00:09 github-actions[bot]

Closing as #847 has been merged 🎉

fmvilas avatar Mar 16 '23 13:03 fmvilas