posit-dev / py-shiny

Shiny for Python
https://shiny.posit.co/py/
MIT License
1.31k stars 79 forks source link

If an nonexistent input is accessed in an output, no error is printed #400

Open wch opened 1 year ago

wch commented 1 year ago

It simply stops execution of the output code without printing an error. For example:

from shiny import App, render, ui

app_ui = ui.page_fluid(
    ui.input_slider("n", "N", 0, 100, 20),
    ui.output_text_verbatim("txt"),
)

def server(input, output, session):
    @output
    @render.text
    def txt():
        return f"n*2 is {input.foo() * 2}"

app = App(app_ui, server)

Live example

wch commented 1 year ago

One problem this causes in practice is that it can be hard to diagnose when the code accidentally accesses a nonexistent input value -- because it's a silent exception, nothing happens, not even a warning or error message.

schloerke commented 1 year ago

Should this return MISSING_TYPE instead? Then an error should be thrown when MISSING_TYPE * 2 is attempted

wch commented 9 months ago

A couple of ideas that came up in a discussion today.

From @schloerke:

If input.n isn't set, it could either:

One issue with both of these approaches is that it may show a flash of red text briefly in the browser, for cases where the input does not exist right away, like with dynamic UI. Currently the app below does not have a red flash, but with both of the approaches above, it would:

from shiny.express import input, render, ui

@render.ui
def foo():
    return ui.input_slider("n", "N", 0, 100, 20)

@render.text
def txt():
    return f"n*2 is {input.n() * 2}"

Another idea, from @cpsievert:

It could continue to raise a silent exception, but a new kind of silent exception where it is logged to the console but not shown in the browser. That way the developer would still be aware of the issue, but the end user would never see a red flash. We could also add more detail to error message telling the developer how to make it go away.

sebovzeoueb commented 6 months ago

Yeah, this one has been annoying me too, in addition to the nonexistent issue, the same kind of thing happens if there's an error while doing some reactive calculation.

If the issue is just that the input isn't set, I think returning None would be fine, the user can handle that as they wish, it's not unusual to check a value's existence in other types of program.

If a reactive calculation (reactive.calc, setting a reactive.value) fails for any reason including not handling an input being None I think it should throw the exception.

EDIT: I am so far unable to create an example that doesn't rely on missing inputs, so this may be the exact issue I've been having too.