decentralized-identity / decentralized-web-node

Decentralized data storage and message relay for decentralized identity and apps.
https://identity.foundation/decentralized-web-node/spec/
Apache License 2.0
402 stars 79 forks source link

`requestId` cannot be a requirement for Response Objects #165

Closed mistermoe closed 2 years ago

mistermoe commented 2 years ago

Regarding response objects, The spec currently states that:

The object MUST include an requestId property, and its value MUST be the [RFC4122] UUID Version 4 string from the requestId property of the Request Object it is in response to.

If the incoming request doesn't have a requestId (which would be a request- level error as defined in section 9.2.3) then the respective response can't have one either.

I'm not certain as to what the benefits/intended purpose behind requestId are, but it seems a bit strange to auto-generate one for a response if the request didn't have one to begin with; especially since it originates from the sender.

@csuwildcat do you remember why you chose to include requestId as a property originating from the sender? I don't suspect it's supposed to act as a nonce. Typically, request ids are generated by a receiving server for the purposes of trace logs. A few practical uses for requestId on both the sending and receiving side would be super helpful.

I'm leaning towards making an argument in favor of either getting rid of them or having them originate from the receiving DWN. Will wait to flesh that out after hearing back from you

Until then, the simplest workaround that comes to mind is to explicitly set requestIdto null in the response if there wasn't one in the respective request

csuwildcat commented 2 years ago

@mistermoe now that we have a nonce requirement for literally all objects, it's probably safe to use drop requestId. The reason we had the sender specify this initially was so async responses could be associated with requests, because how else would a sender know what any random response payload was in relation to?

mistermoe commented 2 years ago

The reason we had the sender specify this initially was so async responses could be associated with requests, because how else would a sender know what any random response payload was in relation to?

@csuwildcat i think async responses as an explicit mechanism will introduce quite a bit of complexity to DWN's design. AFAICT, We don't lose any functionality by making all responses synchronous.

Let's walk through a simple example where asynchrony seems necessary: A chat app. Decentralized signal.

Alice sends Bob a message. Bob's reply to Alice's message shouldn't be considered as the "response" to the request sent to Bob's DWN by Alice's chat app. The response would be 201: Created or 202: accepted indicating that Bob's DWN accepted and stored the message.

Bob's reply (should he ever choose to do so) would be a separate request to Alice's DWN. The message in that request (likely a ThreadsReply) would refer to the appropriate threadId anyway.

I just can't seem to find any real benefit to using a Response object to send a request that contains a reply to a previous message over simply sending a brand new request.

This negates the need for including requestId and messageId in responses which also means we don't run into weird situations where we have to decide what to use for requestId or messageId for malformed requests or messages e.g.

{
  "target": "did:jank:stankybeef",
  "messages": [3]
}

what would we use for the requestId here? and if we figured that out, what would we use for messageId in order to respond to the malformed message 3?

csuwildcat commented 2 years ago

Both requestId and messageId have been eliminated in favor of a simplified approach to sync request/response. This is reflected in the current version of the spec.

mweel1 commented 2 years ago

I really feel this needs be an asynchronous process, and not attacking this now will be a major roadblock in the future.

If done right it will feel synchronous when all parties are available.

There are many use cases where asynchronous is a must, for example..

If your going to a doctor next week and make an appt, he might request your medical records anytime between scheduling the appt. and actually showing up to review them.