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

evaluate failure doesn't return descriptive error message #131

Closed
iamkelllly opened this issue Oct 1, 2021 · 5 comments
Closed

evaluate failure doesn't return descriptive error message #131

iamkelllly opened this issue Oct 1, 2021 · 5 comments
Labels
dev experience Targets the developer experience

Comments

@iamkelllly
Copy link
Contributor

after running fidesctl reset-db, attempted to evaluate resources, and encountered the following:

Loading resource manifests from: demo_resources/
Taxonomy successfully created.
----------
Processing dataset resources...
Traceback (most recent call last):
  File "/usr/local/bin/fidesctl", line 33, in <module>
    sys.exit(load_entry_point('fidesctl', 'console_scripts', 'fidesctl')())
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/click/decorators.py", line 21, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/fides/fidesctl/src/fidesctl/cli/cli.py", line 97, in evaluate
    _apply.apply(
  File "/fides/fidesctl/src/fidesctl/core/apply.py", line 125, in apply
    server_resource_list = get_server_resources(
  File "/fides/fidesctl/src/fidesctl/core/api_helpers.py", line 28, in get_server_resources
    [
  File "/fides/fidesctl/src/fidesctl/core/api_helpers.py", line 29, in <listcomp>
    get_server_resource(
  File "/fides/fidesctl/src/fidesctl/core/api_helpers.py", line 53, in get_server_resource
    raw_server_response: Dict = api.get(
  File "/usr/local/lib/python3.8/site-packages/requests/models.py", line 900, in json
    return complexjson.loads(self.text, **kwargs)
  File "/usr/local/lib/python3.8/json/__init__.py", line 357, in loads
    return _default_decoder.decode(s)
  File "/usr/local/lib/python3.8/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/lib/python3.8/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

After running init-db again, the evaluate succeeded.

This issue is to ensure that we provide a more descriptive error back to the user as to why it's failing.

@iamkelllly iamkelllly added the dev experience Targets the developer experience label Oct 1, 2021
@earmenda
Copy link
Contributor

earmenda commented Oct 1, 2021

This is related to #128 but we saw this happen for an unknown reason. It makes sense that there is a failure after reset-db but this can happen for other reasons which weren't obvious while testing. We should at least document places where we see ugly stack traces and address them.

@ThomasLaPiana
Copy link
Contributor

for sure, this happens when the JSON returned from the webserver doesn't exist or is malformed. We need better error handling in the functions that touch the API, and the API needs better internal error handling as well

@NevilleS
Copy link
Contributor

I've seen maybe 5+ different people hit this error and get stuck; most commonly the issue is they've not run init-db after cleaning their db volume, or similar.

Would like to see us display a more helpful error and figure out how to pre-emptively catch un-initialized DBs, etc.

@ThomasLaPiana
Copy link
Contributor

@NevilleS i think error handling on the webserver side would be best. if it tries to query a table that doesn't exist we can try to catch that error and surface it as "that table doesn't exist, have you initialized the database?"

@ThomasLaPiana
Copy link
Contributor

this will get solved by #204

it will prevent the db from ever not being up-to-date, as the webserver will do it on startup. feel free to reopen if this fix doesn't feel like it is robust enough (general better error handling at the API level is still on the docket)

@iamkelllly iamkelllly removed this from the Roadmap milestone Nov 17, 2021
ThomasLaPiana pushed a commit that referenced this issue Aug 17, 2022
* Add foundation to make Redshift access request queries.

* Fix connection config and dataset config tests that look at a new dataset being added.

* Add support for performing redshift erasures.

* Only mask state field in erasure test - contact is too broad, and was preventing the customer record from getting cleaned up.

* Remove unnecessary assertion and access values as attributes of RowProxy not tuples.

* Add Amazon Redshift example to docs.

* Remove breakpoint, argh.

* Use SQLAlchemy TextClause instead of passing in raw string.

* Add request attribute to mask_data and pass into generate_update_stmt.
ThomasLaPiana pushed a commit that referenced this issue Sep 26, 2022
* Add foundation to make Redshift access request queries.

* Fix connection config and dataset config tests that look at a new dataset being added.

* Add support for performing redshift erasures.

* Only mask state field in erasure test - contact is too broad, and was preventing the customer record from getting cleaned up.

* Remove unnecessary assertion and access values as attributes of RowProxy not tuples.

* Add Amazon Redshift example to docs.

* Remove breakpoint, argh.

* Use SQLAlchemy TextClause instead of passing in raw string.

* Add request attribute to mask_data and pass into generate_update_stmt.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev experience Targets the developer experience
Projects
None yet
Development

No branches or pull requests

4 participants