Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error Handling #200

Open
danielpeintner opened this issue Dec 16, 2019 · 20 comments
Open

Error Handling #200

danielpeintner opened this issue Dec 16, 2019 · 20 comments
Labels
priority: high Issues that will be tackled in 2024 use case Describes a scenario that may be useful for technical decisions

Comments

@danielpeintner
Copy link
Contributor

The scripting API makes use of errors being available in ECMAScript (e.g., TypeError).

Moreover, also other errors such as NotSupportedError or NotFoundError are used which are not available in ECMAScript. Instead those errors are defined here.

We need to have a good set of errors that work for JavaScript/Typescript and browser environments.

For example, NotSupportedError and NotFoundError need to be defined in TypeScript definitions before they can be used in TS.

@egekorkan
Copy link
Contributor

I would also imagine that some of these errors can be mapped to protocols that can be then described in the Binding Templates document. This way, when there is a Consumer and an Exposed Thing communicating, even if these new errors are not sent in the application layer, the errors that are actually sent in the application layer can be translated back into these new errors in the scripting layer.

For example, a consumer sends a readproperty request (GET) over HTTP to a non-existing property in the ExposedThing (even if it is shown to be available in the TD), the Exposed Thing responds with 404 Not Found. This is then translated into NotFoundError in the Consumer runtime. I think that even without Scripting there can be inconsistencies, i.e. maybe some implementations return 403 in this case. So we should test in a plugfest whether everyone is sending the appropriate status codes.

Even further, I think that this can be a good motivation for why one should use the Scripting API in the Exposed Thing. If you don't, then you have to know what status code to send for each protocol.

@zolkis
Copy link
Contributor

zolkis commented Dec 16, 2019

There is (and should be) a difference between API object errors (SecurityError, TypeError etc) and errors reported by protocols in WoT interaction patterns. We need to track that across the whole API.

IMHO there should NOT be error translation between protocols and Scripting objects for a few reasons:

  • data exposed by errors in a browser web API should be minimized against fingerprinting
  • browser web API objects should not invent new error types, but as lined out in the TAG design principles, should use ECMAScript errors (exposed as WebIDL Error) or DOMException
  • but there are a lot of possible error formats and codes in protocols.

Therefore, errors reported via WoT interactions should not be translated, but should be reported via mechanisms embedded into those interactions.

Then, another problem was that TypeScript lacks some of the error definitions: but in Node and TypeScript those could be defined (the restriction only applies to browser APIs).

@zolkis
Copy link
Contributor

zolkis commented Dec 17, 2019

So, we can use the following:

TypeError, RangeError, EvalError, ReferenceError, URIError

Note that SyntaxError and Error are reserved for the ECMAScript parser.

If you think that is enough to map (most) protocol errors, we could try that. However, I think it would be better if interactions had an embedded mechanism for error reporting.

@egekorkan
Copy link
Contributor

Well generally if someone properly implements a generic HTTP server, it would return the proper HTTP codes. For already existing devices, there should thus be a way to inform in the Consumer script the error codes in a non-HTTP way. Of course, we should not try to map all the errors but the most common ones, i.e. no need to map HTTP 418 I'm a teapot status code

@egekorkan
Copy link
Contributor

A table like the following can be used. Maybe we can group them according to WoT operations?

readProperty( )

WoT Error Type Protocol Error/Status Code Description
NotFound 404 the property is not offered by the Thing instance (request sent)

invokeaction( )

WoT Error Type Protocol Error/Status Code Description
NotFound 404 the action is not offered by the Thing instance (request sent)
TypeError 403 bad payload content according to DataSchema

@zolkis
Copy link
Contributor

zolkis commented Feb 24, 2020

no need to map HTTP 418 I'm a teapot status code

Well, of course we need to map that! Everyone supports it! :)

Would the table above be informative or normative when included in the spec?

@mmccool
Copy link
Contributor

mmccool commented Feb 24, 2020

I think we do need a catch-all case if an error code is received for which there is not a definition in the protocol binding. Should this be generic (eg NotFound for any 4xx error code) or should a special error (eg OperationError) for errors which are not defined in the protocol binding?

@zolkis
Copy link
Contributor

zolkis commented Mar 2, 2020

Possible error causes currently used in Scripting:

  • fetching a TD: error comes from the fetch() API
  • consume(): SecurityError and TypeError for failing to validating the TD
  • produce(): SecurityError (TD expansion don't throw ATM and no check is done!) - potential issue.
  • discover(): SecurityError, NotSupportedError, SyntaxError (fetch), DiscoveryError (and set error.message to the code or message from the protocol binding).
  • Consume.read: SecurityError, SyntaxError, error received from protocol bindings
  • Consume.readMultiple: SecurityError, SyntaxError, NotSupportedError (issue!)
  • Consume.readAll: SecurityError, SyntaxError, NotSupportedError, error received from protocol bindings.
  • consume.write: SecurityError, SyntaxError, error received from protocol bindings
  • consume.writeMultiple: SecurityError, SyntaxError, NotSupportedError, error received from protocol bindings.
  • Consume.observe: SecurityError, TypeError, error received from protocol bindings.
  • Consume.unobserve: SecurityError, error received from protocol bindings.
  • Consume.invoke: SecurityError, error received from protocol bindings.
  • Consume.subscribe: SecurityError, TypeError, error received from protocol bindings.
  • Consume.unsubscribe: SecurityError, error received from protocol bindings
  • ExposedThing handling write requests may return ReferenceError, NotSupportedError.
  • ExposedThing handling read requests may return ReferenceError, NotSupportedError.
  • ExposedThing handling action requests may return ReferenceError, NotSupportedError.
  • ExposedThing emit may throw SecurityError, NotFoundError.
  • ExposedThing expose() may reject with SecurityError, TypeError, error received from protocol bindings.
  • ExposedThing.destroy() may reject with SecurityError and error received from protocol bindings.

Some of these need alignment and/or review.
Also, we might factor out an algorithm for reporting an error from protocol bindings.

@egekorkan
Copy link
Contributor

Some feedback to the comment above:

  • fetch API: Fetch is mostly used for HTTP but from what I understand, it is not limited to HTTP. There might be a need to extend it?
  • NotSupportedError for all Consume.x kind of methods should be added.
  • Consume.readMultiple : why there is no "error receuved from protocol bindings"?
  • Consume.unsubscribe : There could be also TypeError since it is possible to supply a payload for event unsubscription
  • Consume.invoke: There could also be TypeError

@zolkis
Copy link
Contributor

zolkis commented Mar 6, 2020

The "fetch a TD" is not part of WoT Scripting API any more: it's "outsourced" to the Fetch API or libraries.

@zolkis
Copy link
Contributor

zolkis commented Jun 18, 2020

Fixed by #218, please check.

@egekorkan
Copy link
Contributor

I do not see the mapping to protocols, is it intended that these covered in the binding templates document? (Or maybe I couldn't find it in the document)

@zolkis
Copy link
Contributor

zolkis commented Jun 19, 2020

Yes, Bindings should provide implementation guidance on mapping to protocols in general, including errors. This is mentioned in [almost all] the algorithms, though in a bit generic way.

Maybe we could add a note or more explicit prose. Keep this open.

@mmccool
Copy link
Contributor

mmccool commented May 6, 2021

During the profiles discussion, we specified a number of HTTP error codes and also the use of the problem details body (RFC7807) if there is a body. The question is, is this information, and indeed the fact that a given protocol is even being used, available to the application upon an error? We may also have two answers to this, depending on whether we are in a privacy-sensitive context or not (or trusted code or not).

I was thinking we could have generic error exception types, and then could pack additional information in the body, including the error code (protocol specific), identification of the protocol, and a copy of the response body. HOWEVER, when the runtime is set up we could specify a "trust level" and if the trust level is "untrusted" then no error details would be provided (just generic error classes).

@zolkis
Copy link
Contributor

zolkis commented May 6, 2021

During the profiles discussion, we specified a number of HTTP error codes and also the use of the problem details body (RFC7807) if there is a body.

Yes, this is useful for the Scripting API implementation.

The question is, is this information, and indeed the fact that a given protocol is even being used, available to the application upon an error?

This is still not fully decided, see https://w3c.github.io/wot-scripting-api/#error-handling (pointing to this issue).
Please provide desired error mappings, per protocol. See comment.

Then, we will have to map (see comment) the errors to Error and DOMException, following the W3C TAG recommendation.
The question is about allowing to convey additional information there.

Right now we do permit that (on pressure from node-wot), like in step 7 of https://w3c.github.io/wot-scripting-api/#the-readproperty-method.
But it's not recommended by the TAG.

@zolkis
Copy link
Contributor

zolkis commented May 6, 2021

I was thinking we could have generic error exception types, and then could pack additional information in the body, including the error code (protocol specific), identification of the protocol, and a copy of the response body. HOWEVER, when the runtime is set up we could specify a "trust level" and if the trust level is "untrusted" then no error details would be provided (just generic error classes).

It is already that way, except that the spec algorithms say where additional error information is allowed to start with.
We can change that in none, or some or all places with steps to determine it based on information obtainable by the implementation (based on security provisioning),

Then the implementation will figure out if it can (from bindings) and allowed (per spec and/or security provisioning) to provide any additional error information. We need to define algorithmic formalizations for this behavior for each use case/interaction.

@zolkis
Copy link
Contributor

zolkis commented May 21, 2021

TODO: define an algorithm for "parse interaction error", similar to parse interaction response, and also a mechanism/algorithm to dispatch/present error data to scripts.

@danielpeintner
Copy link
Contributor Author

Scripting Call 2021-08-02

  • Is it a good design decision to specify an appropriate error message?
    e.g., the error should contain binding information (e.g., formIndex)
  • privacy concerns?

@danielpeintner
Copy link
Contributor Author

related #334

Do we need a proper table like proposed here

@relu91 relu91 added use case Describes a scenario that may be useful for technical decisions priority: high Issues that will be tackled in 2024 labels Jan 15, 2024
@relu91
Copy link
Member

relu91 commented Mar 18, 2024

Call 2024-03-18:

  • @relu91 we should start working on this, one step forward would be to start listing the error that we expect
  • @ashimura there is also an external world that might fire errors that need to be modeled. Also, what are the effects of those errors WoT users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high Issues that will be tackled in 2024 use case Describes a scenario that may be useful for technical decisions
Projects
None yet
Development

No branches or pull requests

5 participants