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

Hard-to-understand error messages from the CLI #130

Closed
gadomski opened this issue Jan 5, 2022 · 7 comments · Fixed by #531
Closed

Hard-to-understand error messages from the CLI #130

gadomski opened this issue Jan 5, 2022 · 7 comments · Fixed by #531
Assignees
Milestone

Comments

@gadomski
Copy link
Member

gadomski commented Jan 5, 2022

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 stac-utils/stactools#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).

@matthewhanson
Copy link
Member

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 matthewhanson added this to the 0.4.0 milestone Apr 19, 2022
@philvarner philvarner modified the milestones: 0.4.0, 0.5.0 Jun 2, 2022
@gadomski gadomski modified the milestones: 0.5.0, 0.6.0 Aug 30, 2022
@gadomski gadomski modified the milestones: 0.6.0, 0.7.0 Jan 27, 2023
@gadomski
Copy link
Member Author

gadomski commented May 4, 2023

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

@ircwaves
Copy link
Member

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

@jsignell
Copy link
Member

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.

@ircwaves
Copy link
Member

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
Copy link
Member

ircwaves commented May 26, 2023

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

@gadomski
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants