pystac-client icon indicating copy to clipboard operation
pystac-client copied to clipboard

Hard-to-understand error messages from the CLI

Open gadomski opened this issue 3 years ago • 1 comments

As raised in #113, it can be hard to know why a given CLI command is failing, as the Exception __str__ may or may not point the user to the source of a problem. E.g., in #113 the type field was incorrect at the /collections endpoint, but you couldn't really tell that from the error message, which was ... big long printout of the collection ... is not a CollectionClient instance.

Similar issues exist, e.g. in stactools https://github.com/stac-utils/stactools/issues/208. At least for the subset of exceptions that are caused by malformed STAC items, we could do some sort of validation or schema checking on the offending item and report where the issue is. This may be something better suited to PySTAC itself (e.g. in a custom Exception type that contains information about why a given JSON object could not be turned into a STAC object).

gadomski avatar Jan 05 '22 13:01 gadomski

Adding this to 0.4.0 to add better error messages from the CLI.

We could leverage PySTAC to provide error messages, however if the errors are due to interacting with the API and parsing response status codes, then pystac-client is probably the better place to handle these.

matthewhanson avatar Apr 19 '22 04:04 matthewhanson

This might be helped/solved by https://github.com/stac-utils/pystac-client/pull/480, we should check.

gadomski avatar May 04 '23 18:05 gadomski

This might be helped/solved by #480, we should check.

This remains an issue. The mildly-mangled /collections response with "type": "collection" leaves pystac-client saying:

[ ... most of a python dict ...] does not represent a CollectionClient instance

ircwaves avatar May 16 '23 15:05 ircwaves

Ian and I talked yesterday about how it is good to include context in error messages, but if the context is a python dict of arbitrary length it would probably be better to truncate or add an ellipsis in the middle of it.

jsignell avatar May 17 '23 15:05 jsignell

So this error message is (mal)formed in pystac. In particular, it is in the Collection.from_dict method. To @gadomski's suggestion, we could employ the pystac.validation.validate_dict call, before attempting construction. That does feel like high overhead, and I lean toward replacing the d in f"{d} is not off type {stac_type}" with d.get("id", "unidentified"), or similar.

I like @jsignell's idea of some ellipsis-truncation better than "unidentified", but I haven't found a light+lazy+informative way to do that yet.

ircwaves avatar May 18 '23 00:05 ircwaves

As it is a cross-repo situation, I didn't flag pystac#1126 as auto-closing this issue, but with a contrived stac-api that contains a bad collection, we now get:

> pystac-client % stac-client collections http://localhost:8000
JSON (id = fake-collection-id) does not represent a CollectionClient instance.

This is better, but I wonder if when pystac raises the error, it should

  1. scoot up the MRO hierarchy until it gets to a foundational class (those defined in the pystac module),
  2. go for the type field of the JSON object first, and fall back to the __name__ of the class.
  3. Nah, this is good enough

ircwaves avatar May 26 '23 13:05 ircwaves

I think for pystac-client it might make sense to catch the STACTypeError and add some extra informative text. Maybe something like:

JSON (id = fake-collection-id) does not represent a CollectionClient instance. The base url http://localhost:8000 could be opened as a STAC Catalog, but the collection at http://localhost:8000/collections/fake-collection-id could not be opened as a STAC Collection.

Or something like that.

gadomski avatar May 26 '23 13:05 gadomski