Define error handler behavior

[lanthaler]

In issue #100 we decided that

RESOLVED: JSON-LD will support a JSON-LD Processor Event mechanism that will report certain events (to be decided later) via a callback given through JSON-LD API calls.

RESOLVED: The JSON-LD Processor Event callback would be registered for every JSON-LD API call, and would provide the type of event and the data associated with the event for the callback. This mechanism would be used to report potential errors, warnings and when the processing of the document was complete.

RESOLVED: When a JSON-LD processor processes input that would result in an exception, it should instead call the JSON-LD Processor Event callback with data concerning the issue that was detected.

We need to clarify that error handler/event callback is supposed to work in detail.

[msporny]

The error handler should probably be something like this:

function(err) {
  // err will have a .type and properties associated with it based on the type

 // return type can be a value or a function(callback)
 //   - value - used when the conflict can be resolved without blocking
 //   - function(callback) - used when a conflict cannot be resolved without blocking
}

For example:

function(err) {
  // auto-correct lists containing other lists by using the first item in the list
  if(err.type === "LISTS_OF_LISTS_DETECTED") {
    return err.list[0];
  }
}

[lanthaler]

I think we will need at least another property in err specifying what's the current operation (compaction, expansion, etc.

Perhaps it would also be easier to have just a data or element property which contains the data which triggered the error instead of having different properties for different error types.

I'm not so sure yet what the return value should be.. The corrected data that will then be processed again? Already processed data (i.e., already expanded, compacted, etc.)? Just a boolean specifying whether to continue or to stop?

[msporny]

Right, so perhaps err would be something like this:

var err = {
  type: 'LIST_OF_LISTS_DETECTED', 
  operation: 'compact', 
  data: {...}, 
  value: [[1,2],3,4]
}

• type - a string denoting the type of error
• operation - a string ('compact', 'expand', 'flatten') denoting the operation that triggered the error
• data - a reference to the complete JSON-LD object being processed as some algorithms may need access to the complete object.
• value - the specific value that must be corrected in order for the algorithm to proceed.

The return type is going to be tricky, we are probably going to have to return an object.

var rval = {
   'continue': true, 
   'value': [1,2],
}

• continue - if true, continue processing. If false, stop.
• value - the corrected value that the algorithm should use to process the data.

[tidoust]

I kind of reacted against the idea that the error handler could run async and take a callback parameter during today's JSON-LD call. In the end, I think I just got confused by terminology.

If I understand things correctly, what we're trying to achieve here is a mechanism for the JSON-LD processor to ask the caller to check and possibly fix an error during processing. That is not so much an error event that gets reported to an error handler as it is a processing step where an error gets passed to an error processor.

If so, I'm entirely ok for this step processor to run async and have a corresponding signature with a callback parameter:

function processError(err, callback) {}

where callback would have the regular node.js callback signature (possible error as first parameter and possible return value to the processor as second parameter).

The term error processor is probably not be the right one and the nuance may be thin. I just think we should avoid calling it an error handler as I wouldn't expect a handler (an error handler in particular) to call a callback.

[msporny]

Thanks for the clarification @tidoust, apologies if I didn't make that clear on the call. I've updated the spec in commit b12d20e to put a specific proposal on the table (as requested by @gkellogg). I also noted the general approach to error handling with an issue marker, since this is just a proposal (but a fairly solid one, imho).

You can see the latest spec text here (Section 3.3.1: JsonLdProcessingError):

http://json-ld.org/spec/latest/json-ld-api/#jsonldprocessingerror

and Section 3.3.4: ConformanceCallback:

http://json-ld.org/spec/latest/json-ld-api/#conformancecallback

[lanthaler]

Manu, if I understand your proposal correctly wouldn't it be the same if we would have an “errorHandling” member in JsonLdOptions where one could e.g. set LIST_OF_LISTS_DETECTED = ignore, or do I miss something? (Yes, you have more data to make that decision but does that actually buy you something?)

The other thing I don’t really understand is what should be put in the “source” member of JsonLdProcessingError. The spec says “An object reference to the original JSON-LD document being processed.” We are modifying the document in-memory so we don’t have a reference to “the original JSON-LD document being processed”. Actually we don’t even have a reference to the root of the document being processed but just a reference to the current element being processed.

I also find the “type” member of JsonLdProcessingError a bit confusing and would propose to rename it to “error”.

Do you have an implementation of this proposal? This would probably help me quite a lot to get my head around this issue.

[lanthaler]

Interesting read: http://www.w3.org/TR/api-design/

[msporny]

Manu, if I understand your proposal correctly wouldn't it be the same if we would have an “errorHandling” member in JsonLdOptions where one could e.g. set LIST_OF_LISTS_DETECTED = ignore, or do I miss something? (Yes, you have more data to make that decision but does that actually buy you something?)

Yes, that's true. There are two ways to go about designing this mechanism. The simpler one is the one you propose above. The only issue I have with that approach is that we close the door on two things if we go that direction; 1) The ability for the developer to do some sort of error logging or forensics on things that they want to log as warnings, but don't necessarily want to stop the processor on, and 2) The ability for us to change the feature in the future such that the document can be "healed" while processing... which might be useful in some situations where you have messy incoming data.

The first can be solved by having a 'halt', 'warn', 'ignore' setting for each error, but that would still require the developer to specify which callback should be called when the 'warn' setting is active. The second is the one that is troubling to me. I'm not positive that we want to prevent that sort of improvement in the future.

The other thing I don’t really understand is what should be put in the “source” member of JsonLdProcessingError. The spec says “An object reference to the original JSON-LD document being processed.” We are modifying the document in-memory so we don’t have a reference to “the original JSON-LD document being processed”. Actually we don’t even have a reference to the root of the document being processed but just a reference to the current element being processed.

We could solve this issue by either; 1) copying the document and operating on the original, returning the copy when there is an issue, or 2) returning the in-process copy. The first would be more correct, but could eat up lots of memory (and be impossible to do in a SAX-like environment. The second would be easier, less memory intensive, less correct, and also impossible to do in a SAX-like environment.

I also find the “type” member of JsonLdProcessingError a bit confusing and would propose to rename it to “error”.

I went back and forth on this one. One view is that it isn't an error yet - only when you tell the processor to halt is it an error. We could say it's a 'conformance warning' and it is only a 'conformance error' if it isn't ignored.

Calling the value 'error' feels a bit strange since it is inside the JsonLdProcessingError dictionary and I would expect that the signature would be callback(error, document), or callback(err, doc). Folks would be typing 'err.error', or 'error.error', which made me cringe a bit.

Do you have an implementation of this proposal? This would probably help me quite a lot to get my head around this issue.

No implementation yet, need to tweak it a bit more before we try one.

[lanthaler]

ACTION: Dave Longley to do an implementation of the conformance issue feature.

[msporny]

@dlongley This issue is blocked on you.

[lanthaler]

PROPOSAL 1 : Simplify the error handling by passing an error object to the callbacks which only consists of an error code and an optional error message containing additional information for debugging.

PROPOSAL 2: Processing is stopped after an error is triggered.

PROPOSAL 3: Introduce a flag which allows processors to continue processing after an error has been reported.

[lanthaler]

PROPOSAL 1 : +1
PROPOSAL 2: +1
PROPOSAL 3: +0 (I would rather not put this in the spec for 1.0 but it would be OK for me if other people think it's a useful thing)

[dlongley]

I'm fine with simplifying the error handler for 1.0; I haven't seen any need to get fancy with it yet. I haven't had the time to work out an implementation of the more complex one anyway. We can look at processor continuation for 1.0+.

PROPOSAL 1: +1
PROPOSAL 2: +1
PROPOSAL 3: -1 (If we really feel we need it, that's ok, but I agree w/Markus that it isn't needed for 1.0)

[tidoust]

PROPOSAL 1: +1
PROPOSAL 2: +1
PROPOSAL 3: -1

[lanthaler]

RESOLVED: Simplify the error handling mechanism by passing an error object to the callbacks which only consists of an error code and an optional error message containing additional information for debugging.

[msporny]

Instead of proposing something in the issue tracker, I found it easier to just work through the text in the spec. Here's what I did:

1. The signatures for all API methods stayed the same.
2. The JsonLdProcessingError was renamed to JsonLdProcessingIssue.
3. JsonLdProcessingIssue gained a member called 'severity', which denotes whether the issue is a warning, recoverable error, or fatal error.
4. The 'callback' parameter for all API methods only ever conveys if there was no error, or a fatal error.
5. If developers want to be notified of warnings and recoverable errors then they can use the issueCallback member in the 'options' parameter to each API method.

I think this balances all of the concerns raised in the http://json-ld.org/minutes/2012-12-11/ telecon and keeps the API nice and clean for those developers that don't want to worry about warnings and recoverable errors. It also aligns the API with the way things are done in node.js callbacks, which has quickly emerged as a JavaScript best-practice. If there are no disagreements with this approach, then we can process the following proposal at the next telecon.

PROPOSAL: Adopt the revised error handler language that was implemented in the JSON-LD API on 2013-05-01.

[gkellogg]

I'm fine with that, but I would like to keep the details of the errors out of the algorithms, which IMO should use words such as SHOULD and MUST to indicate conditions for generating warnings and errors. For example, the current Expansion 3.3 line could be re-written as

3.3) If the active property's container is set to @list, the expended item MUST NOT be an area or list object.

The API can then say:

In step 3.3 of Expansion, if the requirement is violated the processor MUST trigger a listOfListDetected exception.

This keeps the pure algorithms divorced from the specific needs of the API, otherwise, it's bleeding the needs of the API into the pure algorithms.

[lanthaler]

Which warnings and recoverable errors do we have? What's the difference between the two?

Sorry, but I still can’t see why we need this feature (in the spec). Of course any implementation is free to add its own logging/debugging mechanism - but do we really need to specify it?

[msporny]

@gkellogg +1 on splitting errors from algorithms.

@lanthaler example of a warning: "You have used a non-lowercase BCP47 language string. All language strings SHOULD be lowercase." Example of a recoverable error: "The term 'foo' is not defined in the active context and is being dropped from the output.". Example of a fatal error: "A list containing a list was detected while converting to RDF. Lists of lists are not supported in JSON-LD 1.0".

[lanthaler]

OK, I see what you mean. Even though I still don’t think that something like this is necessary. Such information is only useful for linters/validators IMHO. What would a user of a regular JSON-LD Processor do with it? The only argument I see for including this is to standardize the “issue codes”. This is a lot of work for no clear advantage. With just this information it wouldn’t be possible to use a JSON-LD processor for a linter anyway so why worry about it?

I still believe it would be better to stick to the simplest possible solution for fatal errors and leave the rest unspecified. Otherwise we risk creating a solution which imposes more limitations than necessary.

Maybe adding a statement to the spec that concrete implementations MAY have additional (optional) items in JsonLdOptions would be all that's required!?

[lanthaler]

RESOLVED: Accept the JSON-LD API spec text on error handling with a few modifications - remove the issueCallback mechanism from JsonLdOptions, remove severity from JsonLdProcessorIssue, rename JsonLdProcessorIssue to JsonLdError.

[lanthaler]

I've updated the error-handling interfacing according the resolution. Unless I hear objections, I will therefore proceed and close this issue in 24 hours.