As a user, I want to receive error messages when an invalid request is submitted to the API

[tloubrieu-jpl]

💪 Motivation

...so that I can more easily correct my request

📖 Additional Details

This is for errors like 404, 406, etc.

⚖️ Acceptance Criteria

Given a deployed API service on http://{host}/ and a missing lidvid in the registry {not_existing} and a response format {response_format} When I perform a request http://{host}/products/{not_existing} with header Accept: {response_format} Then I expect the response status is 404 and the response contains a message saying "The lidvid {not_existing} was not found"

The request should work with all the {response_format} supported by the application (application/json, application/xml, ...), see documentation on http://{host}/ for the list of supported formats:

⚙️ Engineering Details

The specification need to be updated with a new error or message model response and the controllers will need to support multiple response objects (but I think we already can do that).

We can start from the errorMessage model found in the swagger specification. It contains the following attributes:

• url
• resource
• query
• message

But I don't remember what resource was for, actually the full format does not make a lot of sense to me now.

We could have:

• type (exception object)
• message

That might be enough to start with. Apparently it is what facebook does (see https://blog.restcase.com/rest-api-error-codes-101/)

[al-niessner]

@tloubrieu-jpl @jordanpadams

This may be require a bit of rework of the swaggerhub yaml. What version should I cut-n-paste my local copies into on swaggerhub?

I went through swagger.yml on main and I found a lot of responses with descriptions but no real meat like an object to return. There are very few errors that use errorMessage.

To start with, I would recommend handling the error response in a single block that can be referenced (like Plural and Singular) and have every endpoint use it. Not sure this is actually possible, but $ref is yaml substitution so it should be doable.

From there it gets harder. Do all errors get returned as HTML independent of mime type? Do we support 3 types HTML, JSON, and XML where one of those is down selected from the mime type where plainish text goes to HTML, JSON stays JSON, and XML stays XML?

While I think this is very important, I do want to point out that this going to change the complexity of output generation. The general complexity is mn where m is the number of various output type and n the number of supported mime types. Currently there are wyriwyg, pds, and pds4 for output types and csv,kvp,html,xml,json,pds4+. It ends up being a lot of engineering.serializer.* classes to handle the conversions. Independent of one real output type (all requests map to HTML) or a one-to-one match of mime types it is going to add complexity beyond one class, function, etc. to do the new work.

[tloubrieu-jpl]

Hi @al-niessner ,

I believe the latest copy of the specifcation on swaggerhub is in 0.5.0-SNAPSHOT.

errorMessage has not been actually used in the API yet. I think I just started that at the beginning of the specification, thinking that will be useful but never really used it.

It is good to handle the error response in a single block.

I am thinking the response should be in the format expected by the user for the successful response. Even the error message could be returned as CSV. That should make that more simple and more predictable by the user ?

Would your last section on the complexity be simplified by my previous statement ?

Thanks

[al-niessner]

@tloubrieu-jpl @jordanpadams

Stack trace or not? I am torn because it would be nice to have when a user reports a problem. It will be the log file either way, BUT log files are tough to maintain and 10 days later when the user sends the error the log file may be long gone. Putting a stack trace in the error makes it messy for the user and leaves some/more room for security problems.

Preferences?

[jordanpadams]

I feel like the cleaner response is the more common approach here? I agree the stacktrace would be nice, but if they file a bug we will hopefully track them down and get their logs before they disappear. Not ideal, but I think it is the better solution for the future.

[al-niessner]

My leanings as well. @tloubrieu-jpl ?

[gxtchen]

@viviant100 @tloubrieu-jpl all three content_type returns 404 but response body is still "No content" instead of "The lidvid {not_existing} was not found" see screen captures in https://cae-testrail.jpl.nasa.gov/testrail/index.php?/tests/view/3696752&group_by=cases:section_id&group_order=asc&group_id=82681

[tloubrieu-jpl]

Hi @gxtchen ,

There is a mistake in the test case https://cae-testrail.jpl.nasa.gov/testrail/index.php?/tests/view/3696751&group_by=cases:section_id&group_order=asc&group_id=82681, the release should be downloaded from https://github.com/NASA-PDS/registry-api/releases instead of https://github.com/NASA-PDS/registry-api-service/releases .

Did you acutally used the registry-api-service repository ? It is obsolete, the repository had been archived and will not be edited anymore.

[gxtchen]

@viviant100 @tloubrieu-jpl My test passed using the registry-api v0.5.0-SNAPSHOT. Previously, I was following the readme in https://github.com/NASA-PDS/registry-api.

[viviant100]

Thanks @gxtchen. Let's always test with the latest SNAPSHOT since the stable release usually goes through I&T already prior to its release.