feat: handle interaction error response algorithm#589
Conversation
Initial draft of the algorithm to handle interaction error response, as described in the issue w3c#560. Referenced in readProperty method.
danielpeintner
left a comment
There was a problem hiding this comment.
Some very high level comments/questions
- Do we need the concept of "interaction method"? Why can't we use
opright away ? I did not quite get that point 🙈 - I think the editors not right before section 8 needs to move up
I needed the new definition to quickly reference all the ConsumedThing methods that implement the WoTInterface here:
Ofc, we can remove it if you have better suggestions.
Yes, that is another discussion point: should we keep it? |
| <p> | ||
| This algorithm is used by <a>interaction methods</a> to process error responses | ||
| from the <a>Protocol Bindings</a> and determine whether to create an | ||
| {{InteractionError}} or map to a standard error type. |
There was a problem hiding this comment.
I was initially confused with the standard error type but read further and understood that it means a DOMException. Am I right?
There was a problem hiding this comment.
I think @relu91 is hinting at the other errors we are using like NotFoundError, SecurityError, NotSupportedError , SyntaxError ...
Correct?
|
From today's call:
I covered the comments and finished referencing the algortihm in relevant section. @danielpeintner and @egekorkan please have a look. |
danielpeintner
left a comment
There was a problem hiding this comment.
Looks very good. Some comments below.
| <h3>The <dfn>InteractionError</dfn> interface</h3> | ||
| <p class="note"> | ||
| Currently, we are looking for wider implementation of the following section. | ||
| The alorithm presented here might be subject to changes in the future. |
There was a problem hiding this comment.
| The alorithm presented here might be subject to changes in the future. | |
| The algorithm presented here might be subject to changes in the future. |
| Return a new {{InteractionError}} constructed with |errorOutput| an appropriate | ||
| message derived from the |binding| or |error|. |
There was a problem hiding this comment.
This sentence reads a bit strange.
What about
Return a new InteractionError constructed with errorOutput. Please ensure the message is appropriately derived from the specific binding or error encountered.
I can only make non-substantial contributions now (as I am not a group member any more). The mismatch comes from the fact the Scripting API is a W3C spec, and as such, uses the Infra. |
| </p> | ||
| <pre class="idl"> | ||
| [SecureContext, Exposed=(Window,Worker)] | ||
| interface InteractionError : Error { |
There was a problem hiding this comment.
WebIDL suggests extending DOMException, I need to update this before merging, since the discussion (Error vs DOMException is still open.
There was a problem hiding this comment.
@danielpeintner turns out that if we want to extend DOMException, the "extra data" we carry in the Exception must be serializable. Sadly, InteractionOutput is not serializable as it contains a non-serializable field ( ReadableStream? data). We can:
- Leave the changes as they are, but we have this inconsistency between the "native" errors and our "custom" error. The inconsistency is that InteractionError does not extend
DOMException. - We force the thrower of the error to eagerly read the full payload and then throw.
There was a problem hiding this comment.
It seems easiest to stick with Error since we are not happy with DOMException anyway.
There was a problem hiding this comment.
It seems we have the same problem with https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error . However, I'm aware that some HTTP clients do not respect the serializable constraint.
There was a problem hiding this comment.
Mhh, what are you proposing?
Initial draft of the algorithm to handle interaction error response, as described in issue #560.
Discussions points:
interaction methodto indicate one of the methods of Consumed thing that implements a TD operation.AdditionalExpectedResponseand the actual response is left underspicified to let the protocol binding implementation decide (for example, the HTTP binding might usehtv:statusCodeValueto match the right expected response)InteractionOutputas I did withInteractionErrorInteractionOuputtake as input a Form we should override its content-type with the one declared in theAdditionalExpectedResponse. This change is not yet in this PR.Preview | Diff