Error conditions and expected results

tomschenkjr commented 7 years ago

Our next milestone for the project is to incorporate more robust error messages when something unexpected happens during the API transaction. As of v0.4.0, we are only testing and expecting complete and correct records. While we've been discussing this, Alliance also mentioned this in #12 as part of feedback on the current API.

I'd like for us to discuss a handful of items before we start work on any implementation for v0.5.0. Namely,

What are the likely scenarios for errors?
What is the expected result for that condition (i.e., errror message)?
What field should the expected result occur (i.e., an existing field, new field, or headers)?

Below are some scenarios that I've initially drawn-up. Some may be missing, so let's identify any new items. Once we're able to fill-out this table, I'll feel comfortable with proceeding. Please provide input and feedback in the comments below.

#	Scenario	Expected Result	Field
1	Non-optional field is missing
2	Incorrect data type
3	Invalid JSON formatting
4	Date is invalid formatting
5	Invalid race codes (but valid data type)
6	Child over 1 year-old (tentative)

tomschenkjr commented 7 years ago

Per the discussion on the phone today, we have proposed a number of items. First, we'll propose an error field that will provide a numeric field that describes the error code. After looking at a few APIs, I propose this is an array that contains both the code and the message. Under this approach, it may be possible to have more than one error (i.e., errors[0], errors[1], etc.) That is, the response field will have the following structure:

"errors": [
    {
         "message": "An error message which conveys details",
         "code":100
    }
 ]

Second, we'll have a list of error codes that correspond to a description of the source of the error. Are there any more types of errors that should be included?

Error Code	Error Name	Error Explanation
0	OK	No error.
100	Data field missing	A required field was not included in the submission.
101	Incorrect data type	A field contained an unexpected data type that does not match submission requirements.
102	Incorrect date format	One of the date formats is incorrect.
103	Invalid JSON	JSON structure was incomplete, contained unexpected characters, or was not properly closed.
104	Invalid race/ethnicity code	Value used to encode race or ethnicity does not match an expected value.
105	Child over 1 years-old	The age of the child was over 1 years-old.
106	Geocoding error	Address submitted is outside Chicago city limits or was not found in the geocoder.

Third, the error code(s) always needs to be included as specified. However, the error message(s) may either repeated the aforementioned error explanation or provide an error message that's passed along from a source. For instance, errors array may contain "code": 106 (geocoding) error while the message may simply forward the original geocoding error.

Finally, I'm cc'ing @AOCjcarr who raised this in #12 into this conversation.

AOCjcarr commented 7 years ago

This approach sounds correct. Would it be safe to say: If no errors 0 and corresponding message will be returned. If multiple errors, then the ‘Message’ and ‘code’ components will repeats as pairs For 101/102/103. Will the message just be generic as shown below, or possible to include which component or specific data the error relates to? Generic is acceptable as there are few variables, however the more specific, will make it much more user friendly.

Thanks, Jeremy Carr 773.580.0099

From: Tom Schenk Jr [mailto:notifications@github.com] Sent: Friday, October 13, 2017 5:15 PM To: Chicago/lead-safe-api-docs lead-safe-api-docs@noreply.github.com Cc: Jeremy Carr jcarr@alliancechicago.org; Mention mention@noreply.github.com Subject: Re: [Chicago/lead-safe-api-docs] Error conditions and expected results (#14)

Per the discussion on the phone today, we have proposed a number of items. First, we'll propose an error_code field that will provide a numeric field that describes the error code. After looking at a few APIs, I propose this is an array that contains both the code and the message. Under this approach, it may be possible to have more than one error (i.e., errors[0], errors[1], etc.) That is, the response field will have the following structure:

{

"errors": [

 {

      "message": "An error message which conveys details",

      "code":100

 }

]

}

Second, we'll have a list of error codes that correspond to a description of the source of the error. Are there any more types of errors that should be included? Error Code

Error Name

Error Explanation

0

OK

No error.

100

Data field missing

A required field was not included in the submission.

101

Incorrect data type

A field contained an unexpected data type that does not match submission requirements.

102

Incorrect date format

One of the date formats is incorrect.

103

Invalid JSON

JSON structure was incomplete, contained unexpected characters, or was not properly closed.

104

Invalid race/ethnicity code

Value used to encode race or ethnicity does not match an expected value.

105

Child over 1 years-old

The age of the child was over 1 years-old.

106

Geocoding error

Address submitted is outside Chicago city limits or was not found in the geocoder.

Third, the error code(s) always needs to be included as specified. However, the error message(s) may either repeated the aforementioned error explanation or provide an error message that's passed along from a source. For instance, errors array may contain "code": 106 (geocoding) error while the message may simply forward the original geocoding error.

Finally, I'm cc'ing @AOCjcarrhttps://github.com/aocjcarr who raised this in #12https://github.com/Chicago/lead-safe-api-docs/issues/12 into this conversation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Chicago/lead-safe-api-docs/issues/14#issuecomment-336580271, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AenUx0nkxFp8RebnPak6Il2sGLYed8jvks5sr-D_gaJpZM4P0hUX.

tomschenkjr commented 7 years ago

If no errors 0 and corresponding message will be returned. If multiple errors, then the ‘Message’ and ‘code’ components will repeats as pairs

That is correct.

Will the message just be generic as shown below, or possible to include which component or specific data the error relates to?

I think it could be fairly customized, but specificity will depend a bit. At the very least, we may be able to pass along the underlying error, which will probably provide some indication on which element/step is causing the error. If it's not clear to you, we should be able to determine the error. @potash - curious on your input on this as well.

potash commented 7 years ago

I agree with @tomschenkjr that we could make the message more specific than the code. For example I don't think we want a separate code for every possible missing data field-- but we can specify in the message which field (or fields) was (were) missing.

geneorama commented 7 years ago

@tomschenkjr where is the documentation for the error codes, and how do we finalize that? I think you have a branch started, but I wanted to check if that's the standard or if there's a wiki or something else.

Thanks

tomschenkjr commented 7 years ago

Pull request #17

From: Gene Leynes notifications@github.com Sent: Friday, November 3, 2017 2:51:51 PM To: Chicago/lead-safe-api-docs Cc: Schenk, Tom; Mention Subject: Re: [Chicago/lead-safe-api-docs] Error conditions and expected results (#14)

@tomschenkjrhttps://github.com/tomschenkjr where is the documentation for the error codes, and how do we finalize that? I think you have a branch started, but I wanted to check if that's the standard or if there's a wiki or something else.

Thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Chicago/lead-safe-api-docs/issues/14#issuecomment-341809203, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABkC0WOAYysa37rCRDz6dL8fLdXvJpAUks5sy27XgaJpZM4P0hUX.

This e-mail, and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail (or the person responsible for delivering this document to the intended recipient), you are hereby notified that any dissemination, distribution, printing or copying of this e-mail, and any attachment thereto, is strictly prohibited. If you have received this e-mail in error, please respond to the individual sending the message, and permanently delete the original and any copy of any e-mail and printout thereof.

tomschenkjr commented 6 years ago

@AOCjcarr today our team was discussing the error you encountered and error messages that we're planning to provide in v0.5.0 of the API. We've begun to questioning of the specific wording that we're providing. Namely, whether we should orient the error messages to be clear to you and your team (techies) versus error messages that are useful for the clinicians.

For example, below is an example of the file you would be receiving in v0.5.0:

...
"errors":
  [
    {
      "message": "A field contained an unexpected data type that does not match submission requirements.",
      "code": 400
    },
    {
       "message": "A field contained an unexpected data type that does not match submission requirements.",
       "code": 400
    },
    {
       "message": "Address is outside of Chicago or address failed to be geocoded within Chicago.",
       "code": 412
    }
  ]

Is the messaging appropriate for your needs? Does it need to be more technical or less technical? We're not sure if this message will be shared with clinicians, or just needs to be targeted for use with your team.

geneorama commented 6 years ago

@tomschenkjr previously I thought that these error codes, now called status codes, would be returned with the status codes in the header.

As I understand it now, these status codes will be returned as an element of the json_to_alliance item (i.e. the main response), and it will be present even when there is no error with a return value of 200 for success.

So, shouldn't we update the example response in the documentation? https://github.com/Chicago/lead-safe-api-docs/blob/dev/docs/index.md

Instead of this:

{
    "version": "0.3.0", 
    "timestamp": "2017-08-11 19:20:29.000-00:00", 
    "network_id": "Alliance Health",
    "clinic_id": "EF",
    "location_id": "examp_loc",
    "patient_id": "9000", 
    "risk_score": "0.309", 
    "risk_score_notes": "Risk Score Notes"
}

I think it should look like this?

{
    "version": "0.3.0", 
    "timestamp": "2017-08-11 19:20:29.000-00:00", 
    "network_id": "Alliance Health",
    "clinic_id": "EF",
    "location_id": "examp_loc",
    "patient_id": "9000", 
    "risk_score": "0.309", 
    "risk_score_notes": "Risk Score Notes",
    "errors": 
    [
        {
          "message": "No error.",
          "code": 200
        }
    ]
}

tomschenkjr commented 6 years ago

Yes, good catch, though I'm not sure if there is an error, whether the rest of the fields will be returned.

It also raises a question for us: should we return errors even if the response is 200 - OK?

Likewise, if there is an error, which fields will be returned along with it? version, timestamp, network_id, patient_id?

@avishekrk - thoughts?

avishekrk commented 6 years ago

My first thought is that we can return all the fields but we can put blank values in them with the exception of the error code when there is an error.

When we have no errors we can leave the error code field blank.

geneorama commented 6 years ago

@avishekrk well, in the docs we have a code for "no errors", so seems we should include that if there are no errors.

Also, to be consistent with documentation, shouldn't we rename the field to "Response Codes" with "Status Class/Code/Message" as subfields?

Also, in the example we are returning the Code and Message, maybe we want the Class as well?

tomschenkjr commented 6 years ago

For now, let's keep the thread focused on the most recent question.

Does Avishek solution makes sense? When there is not an error, does it make sense to not return the 200-OK (thus, changing the current plan)?

From: Gene Leynes notifications@github.com Sent: Tuesday, December 5, 2017 1:55:44 PM To: Chicago/lead-safe-api-docs Cc: Schenk, Tom; Mention Subject: Re: [Chicago/lead-safe-api-docs] Error conditions and expected results (#14)

@avishekrkhttps://github.com/avishekrk well, in the docs we have a code for "no errors", so seems we should include that if there are no errors.

Also, to be consistent with documentation, shouldn't we rename the field to "Response Codes" with "Status Class/Code/Message" as subfields?

Also, in the example we are returning the Code and Message, maybe we want the Class as well?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Chicago/lead-safe-api-docs/issues/14#issuecomment-349422003, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABkC0R9Ve3aqY9tqcYpMhQiquFBK9z4Bks5s9Z_AgaJpZM4P0hUX.

This e-mail, and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail (or the person responsible for delivering this document to the intended recipient), you are hereby notified that any dissemination, distribution, printing or copying of this e-mail, and any attachment thereto, is strictly prohibited. If you have received this e-mail in error, please respond to the individual sending the message, and permanently delete the original and any copy of any e-mail and printout thereof.

Chicago / lead-safe-api-docs

Error conditions and expected results #14