Skip to content

Conversation

@tconkling
Copy link
Contributor

@tconkling tconkling commented Oct 21, 2019

  • Defines a new exception class, StreamlitAPIException, that's used for exceptions resulting from improper usage of the streamlit API. (That is, StreamlitAPIException is meant for "user errors", like calling a DeltaGenerator function with bad inputs.)
  • Exception.proto has a new field, message_is_markdown, which is True when the exception being sent is a StreamlitAPIException instance. Markdown formatting in the exception message will be nicely rendered on the frontend.
  • StreamlitAPIException stack traces get all Streamlit entries pruned before being marshalled into the exception proto. This means that when a user misuses a Streamlit API, they no longer see a bunch of ScriptRunner and DeltaGenerator lines in the stack trace we show them.
  • I updated most (all?) user-facing errors to use StreamlitAPIException, for the nicer formatting.
  • Non-API-related errors, originating from the bowels of Streamlit, will not be specially pruned/formatted, so this shouldn't hinder us when debugging Streamlit itself.

For example, here is what the "DuplicateWidgetID" error used to look like:

Screen Shot 2019-10-21 at 1 46 39 PM

And with these changes, this is what it looks like now:

image

Fixes #438

@tconkling tconkling requested review from a team and removed request for a team October 21, 2019 20:52
@tconkling tconkling added the WIP label Oct 21, 2019
* develop:
  Make pre/code/a look good in Alert even outside Markdown block (streamlit#480)
@tconkling tconkling requested a review from a team as a code owner October 21, 2019 21:39
@tconkling tconkling removed the WIP label Oct 21, 2019
Copy link
Contributor

@tvst tvst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed everything except lib/streamlit/elements/exception_proto.py. Will look at that later.

@tvst
Copy link
Contributor

tvst commented Oct 24, 2019

streamlit run e2e/scripts/exception.py looks funny:

image


.exception .message code {
color: $primary;
}
Copy link
Contributor

@tvst tvst Oct 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this color looks funny. You didn't like the green?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the green-on-red kinda looks like a tacky Christmas tree:

image

vs

image

(But I'll back out the change!)

LOGGER = get_logger(__name__)


_streamlit_dir = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't you initialize this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just being overly paranoid about import time. I'll simplify.

* develop:
  Use custom context manager for temporary file in cli.py (streamlit#489)
  Update pull_request_template.md
  Update pull_request_template.md
  Handle lack of trailing slash (streamlit#528)
  Set auto_envvar_prefix to STREAMLIT (streamlit#477)
  Strip slashes in path components (streamlit#523)
  Create doc_improvement.md
  ESlint: allow @ts-ignore (streamlit#499)
  Update Pipfile (streamlit#505)
  Release 0.49.0 (streamlit#509)
  Removing package json (streamlit#501)
  Feature/input number (streamlit#416)
  ESLint warnings are fatal in Circle (streamlit#492)
  Doc updates (streamlit#286)
@tconkling tconkling requested review from a team and tvst October 29, 2019 23:04
* develop:
  Fix cypress tests (streamlit#559)
  Extends capability to send config keys by param to "streamlit hello" command (streamlit#527)
  Explicitly use bash in Makefile (streamlit#556)
  Fix trailing whitespace (streamlit#562)
  Adding missing key parameter to text_input (streamlit#571)
  Create CONTRIBUTING.md
  Don't resize vega chart on update (streamlit#497)
  LaTeX support (streamlit#491)
  Workaround for deep import reloads not working (streamlit#537)
  Add dist-packages and site-packages to blacklist just in case. (streamlit#544)
messageNode = <StreamlitMarkdown source={markdown} allowHTML={false} />
} else {
messageNode = (
<>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tconkling, can you use Fragment instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding what you're asking, but <> is a shorthand for <React.Fragment>: https://reactjs.org/blog/2017/11/28/react-v16.2.0-fragment-support.html.

Copy link
Contributor

@tvst tvst Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, I think @kantuni told me a few times to move from <Fragment> to <>? Or was it someone else? 😄

Personally, I have no horse in this race. And I don't even care about keeping a unified style in our codebase for this, except for: it should be the same style within single file.

Copy link
Contributor Author

@tconkling tconkling Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it seems to be the preferred shorthand for Fragment, I'm going to stick with <> until someone tells me otherwise :)

let stackTraceNode: ReactNode = null
if (stackTrace && stackTrace.size > 0) {
stackTraceNode = (
<>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@tconkling tconkling merged commit 84c683d into streamlit:develop Nov 4, 2019
@tconkling tconkling deleted the tim/StreamlitException branch November 4, 2019 18:56
sthagen added a commit to sthagen/streamlit-streamlit that referenced this pull request Nov 4, 2019
Nicer frontend exception display (streamlit#490)
tvst pushed a commit that referenced this pull request Nov 11, 2019
- Defines a new exception class, `StreamlitAPIException`, that's used for exceptions resulting from improper usage of the streamlit API. (That is, `StreamlitAPIException` is meant for "user errors", like calling a DeltaGenerator function with bad inputs.)
- `Exception.proto` has a new field, `message_is_markdown`, which is True when the exception being sent is a `StreamlitAPIException` instance. Markdown formatting in the exception message will be nicely rendered on the frontend.
- `StreamlitAPIException` stack traces get all Streamlit entries pruned before being marshalled into the exception proto. This means that when a user misuses a Streamlit API, they no longer see a bunch of ScriptRunner and DeltaGenerator lines in the stack trace we show them.
- I updated most (all?) user-facing errors to use StreamlitAPIException, for the nicer formatting.
- Non-API-related errors, originating from the bowels of Streamlit, will _not_ be specially pruned/formatted, so this shouldn't hinder us when debugging Streamlit itself.

Fixes #438
tvst added a commit that referenced this pull request Nov 11, 2019
* Create CONTRIBUTING.md

* Adding missing key parameter to text_input (#571)

* Adding key parameter to number_input

* Test

* linter

* Fix trailing whitespace (#562)

* Explicitly use bash in Makefile (#556)

The Makefile uses bash-isms like `[[ $(PY_VERSION) == "3.6.0" || $(PY_VERSION) > "3.6.0" ]]`
that don't work in POSIX sh.  So let's be explicit.

This fixes a problem with the build in Ubuntu, where you currently get `[[: not found`.

* Extends capability to send config keys by param to "streamlit hello" command (#527)

* add parameter config options to "streamlit hello"

* added param config keys to "config show" command

* fix lint

* add custom env var names for streamlit cmd options

* cleanup commit

* Fix cypress tests (#559)

* update disconnected snapshot

* update empty_datafrasmes snapshots

* fix pyplot_kwargs cypress test

* fix Cypress tests: return all promises based image comparisons so IO does not conflict

* fix typo on vega_lite_chart.spec.ts

* remove ribbon decoration from the top during e2e tests

* restore load check to pyplot cypress test

* remove ribbon from add_rows and pyplot

* Nicer frontend exception display (#490)

- Defines a new exception class, `StreamlitAPIException`, that's used for exceptions resulting from improper usage of the streamlit API. (That is, `StreamlitAPIException` is meant for "user errors", like calling a DeltaGenerator function with bad inputs.)
- `Exception.proto` has a new field, `message_is_markdown`, which is True when the exception being sent is a `StreamlitAPIException` instance. Markdown formatting in the exception message will be nicely rendered on the frontend.
- `StreamlitAPIException` stack traces get all Streamlit entries pruned before being marshalled into the exception proto. This means that when a user misuses a Streamlit API, they no longer see a bunch of ScriptRunner and DeltaGenerator lines in the stack trace we show them.
- I updated most (all?) user-facing errors to use StreamlitAPIException, for the nicer formatting.
- Non-API-related errors, originating from the bowels of Streamlit, will _not_ be specially pruned/formatted, so this shouldn't hinder us when debugging Streamlit itself.

Fixes #438

* #558 fix bart_vs_bikes example (#599)

* Error is an undefined name in run_on_save.py (#605)

python3 -c "Exception"  # No problems
python3 -c "NotImplementedError"  # No problems
python3 -c "Error"  # NameError: name 'Error' is not defined

[flake8](http://flake8.pycqa.org) testing of https://github.com/streamlit/streamlit on Python 3.8.0

$ __flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics__
```
./lib/tests/streamlit/scriptrunner/test_data/compile_error.py:22:9: E999 SyntaxError: invalid syntax
because i am a compile error!
        ^
./lib/streamlit/hashing.py:118:32: F821 undefined name 'string_types'
            or isinstance(obj, string_types)
                               ^
./lib/streamlit/hashing.py:255:34: F821 undefined name 'string_types'
            elif isinstance(obj, string_types):
                                 ^
./lib/streamlit/hashing.py:306:42: F821 undefined name 'string_types'
                or (isinstance(obj.name, string_types) and os.path.exists(obj.name))
                                         ^
./lib/streamlit/hashing.py:394:34: F821 undefined name 'string_types'
            if not isinstance(n, string_types) or not n.endswith(".<lambda>")
                                 ^
./lib/streamlit/__init__.py:547:14: F821 undefined name 'source_util'
        with source_util.open_python_file(filename) as source_file:
             ^
./lib/streamlit/caching.py:81:18: F821 undefined name 'Dict'
_mem_cache = {}  # type: Dict[string, CacheEntry]
                 ^
./lib/streamlit/caching.py:81:18: F821 undefined name 'string'
_mem_cache = {}  # type: Dict[string, CacheEntry]
                 ^
./lib/streamlit/hello/demos.py:213:9: F821 undefined name 'reload'
        reload(sys)
        ^
./lib/streamlit/elements/vega_lite.py:41:22: F821 undefined name 'dict_types'
    if type(data) in dict_types and spec is None:
                     ^
./lib/streamlit/elements/vega_lite.py:89:31: F821 undefined name 'dict_types'
        if type(data_spec) in dict_types:
                              ^
./lib/streamlit/elements/media_proto.py:76:22: F821 undefined name 'string_types'
    if type(data) in string_types:
                     ^
./lib/streamlit/elements/media_proto.py:122:25: F821 undefined name 'string_types'
    if isinstance(data, string_types) and url(data):
                        ^
./lib/streamlit/elements/media_proto.py:154:25: F821 undefined name 'string_types'
    if isinstance(data, string_types) and url(data):
                        ^
./lib/streamlit/elements/lib/dicttools.py:125:30: F821 undefined name 'native_dict'
        if type(v) in (dict, native_dict):
                             ^
./lib/streamlit/elements/lib/dicttools.py:129:42: F821 undefined name 'native_dict'
                if type(child) in (dict, native_dict):
                                         ^
./docs/conf.py:206:41: F821 undefined name 'github_doc_root'
            "url_resolver": lambda url: github_doc_root + url,
                                        ^
./docs/api-examples-source/widget.time_input.py:3:39: F821 undefined name 'datetime'
t = st.time_input("Set an alarm for", datetime.time(8, 45))
                                      ^
./examples/run_on_save.py:68:11: F821 undefined name 'Error'
    raise Error("Windows not supported")
          ^
./examples/run_on_save.py:71:11: F821 undefined name 'Error'
    raise Error("Unknown platform")
          ^
1     E999 SyntaxError: invalid syntax
19    F821 undefined name 'datetime'
20
```
__E901,E999,F821,F822,F823__ are the "_showstopper_" [flake8](http://flake8.pycqa.org) issues that can halt the runtime with a SyntaxError, NameError, etc. These 5 are different from most other flake8 issues which are merely "style violations" -- useful for readability but they do not effect runtime safety.
* F821: undefined name `name`
* F822: undefined name `name` in `__all__`
* F823: local variable name referenced before assignment
* E901: SyntaxError or IndentationError
* E999: SyntaxError -- failed to compile a file into an Abstract Syntax Tree

* Undefined name: import datetime in widget.time_input.py (#606)

[flake8](http://flake8.pycqa.org) testing of https://github.com/streamlit/streamlit on Python 3.8.0

$ __flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics__
```
./lib/tests/streamlit/scriptrunner/test_data/compile_error.py:22:9: E999 SyntaxError: invalid syntax
because i am a compile error!
        ^
./lib/streamlit/hashing.py:118:32: F821 undefined name 'string_types'
            or isinstance(obj, string_types)
                               ^
./lib/streamlit/hashing.py:255:34: F821 undefined name 'string_types'
            elif isinstance(obj, string_types):
                                 ^
./lib/streamlit/hashing.py:306:42: F821 undefined name 'string_types'
                or (isinstance(obj.name, string_types) and os.path.exists(obj.name))
                                         ^
./lib/streamlit/hashing.py:394:34: F821 undefined name 'string_types'
            if not isinstance(n, string_types) or not n.endswith(".<lambda>")
                                 ^
./lib/streamlit/__init__.py:547:14: F821 undefined name 'source_util'
        with source_util.open_python_file(filename) as source_file:
             ^
./lib/streamlit/caching.py:81:18: F821 undefined name 'Dict'
_mem_cache = {}  # type: Dict[string, CacheEntry]
                 ^
./lib/streamlit/caching.py:81:18: F821 undefined name 'string'
_mem_cache = {}  # type: Dict[string, CacheEntry]
                 ^
./lib/streamlit/hello/demos.py:213:9: F821 undefined name 'reload'
        reload(sys)
        ^
./lib/streamlit/elements/vega_lite.py:41:22: F821 undefined name 'dict_types'
    if type(data) in dict_types and spec is None:
                     ^
./lib/streamlit/elements/vega_lite.py:89:31: F821 undefined name 'dict_types'
        if type(data_spec) in dict_types:
                              ^
./lib/streamlit/elements/media_proto.py:76:22: F821 undefined name 'string_types'
    if type(data) in string_types:
                     ^
./lib/streamlit/elements/media_proto.py:122:25: F821 undefined name 'string_types'
    if isinstance(data, string_types) and url(data):
                        ^
./lib/streamlit/elements/media_proto.py:154:25: F821 undefined name 'string_types'
    if isinstance(data, string_types) and url(data):
                        ^
./lib/streamlit/elements/lib/dicttools.py:125:30: F821 undefined name 'native_dict'
        if type(v) in (dict, native_dict):
                             ^
./lib/streamlit/elements/lib/dicttools.py:129:42: F821 undefined name 'native_dict'
                if type(child) in (dict, native_dict):
                                         ^
./docs/conf.py:206:41: F821 undefined name 'github_doc_root'
            "url_resolver": lambda url: github_doc_root + url,
                                        ^
./docs/api-examples-source/widget.time_input.py:3:39: F821 undefined name 'datetime'
t = st.time_input("Set an alarm for", datetime.time(8, 45))
                                      ^
./examples/run_on_save.py:68:11: F821 undefined name 'Error'
    raise Error("Windows not supported")
          ^
./examples/run_on_save.py:71:11: F821 undefined name 'Error'
    raise Error("Unknown platform")
          ^
1     E999 SyntaxError: invalid syntax
19    F821 undefined name 'datetime'
20
```
__E901,E999,F821,F822,F823__ are the "_showstopper_" [flake8](http://flake8.pycqa.org) issues that can halt the runtime with a SyntaxError, NameError, etc. These 5 are different from most other flake8 issues which are merely "style violations" -- useful for readability but they do not effect runtime safety.
* F821: undefined name `name`
* F822: undefined name `name` in `__all__`
* F823: local variable name referenced before assignment
* E901: SyntaxError or IndentationError
* E999: SyntaxError -- failed to compile a file into an Abstract Syntax Tree

* Define github_doc_root in docs/conf.py (#608)

Fixes: #607

* Fix matplotlib backend on all platforms (#595)

* PlotlyChart: Force recreation of <Plot> in render (#629)

Plotly charts are not always re-rendering properly. 

This seems to be a react-plotly bug; I made a standalone repro case and submitted it to them here plotly/react-plotly.js#167.

Fixes #512

* Update bug_report.md

* Update feature_request.md

* Replace fetch with axios in JS websocket code (#598)

* Using Axios

* Checking timeout

* lock file

* Cleaning stale elements when the reportHash changes (#617)

* Cleaning stale elements when the reportHash changes

* Cleaning

* Fixing Metrics

* Removing reportName and some cleaning

* fix for #618: pandas and numpy import unification in examples (#633)

* Fixing print as pdf (#566)

* Fixing print as pdf

* Fixing firefox print

* Fixing double font for firefox

* Support GitHub blobs and gists by transforming normal to "raw" URLs. (#645)

* starting conditions from @aritropaul's PR

* added test scaffolding

* snakecase and mild refactor of function

* refactor into util.py

* added fixture structure

* Update bug_report.md

* Update feature_request.md

* rebasing cli.py to streamlit/develop to fix missing commits

* linting

* fleshed out the tests. should collect more fixtures.

* working regular expression and tests. needs confirmation.

* copied in yarn.lock from streamlit/streamlit/develop

* make sure not to spuriously replace "blob", eg. username "theblob"

* mild function reorg and comment style change

* Undefined names: missing imports (#632)

* Undefined names: missing imports

* noqa: F821 because linters don't like magic

* Only install watchdog on Mac if gcc is working (#587)

* check if gcc is working before making watchdog a dependency

* black

* Pull these conditions into their own boolean constants, for readability:

* only check for xcode if we're on darwin

* Adding sidebar to main concepts (#507)

* Adding sidebar to main concepts.

* Draft of sidebar content.

* Addressing Thiago's comments.

* report -> app.

* Addressing Thiago's comment in PR507.

* Fix empty_charts example

* Improve docs about LaTeX

* Update examples in DeltaGenerator docs and api.md

* Fix embed=true padding and scrollbars

* Fix wording in "App snapshot" dialog

* Version 0.50.0

* Improve how exception tracebacks render

* Version 0.50.1

* Changelog for 0.50.1

* Make CSS changes to stack traces only apply to stack traces -- not any pre/code in an stException

* Version 0.50.2

* Fix date in changelog :D

* Update notices.
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 this pull request may close these issues.

Support markdown in user-facing exceptions

3 participants