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.
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