Skip to content

Commit

Permalink
Better messages for AttributeError in Shiny Express (#888)
Browse files Browse the repository at this point in the history
  • Loading branch information
wch authored Dec 11, 2023
1 parent bac1c48 commit 6a28a59
Showing 1 changed file with 53 additions and 40 deletions.
93 changes: 53 additions & 40 deletions shiny/express/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,51 +72,64 @@ def set_result(x: object):
nonlocal ui_result
ui_result = cast(Tag, x)

prev_displayhook = sys.displayhook
sys.displayhook = set_result

reset_top_level_recall_context_manager()
get_top_level_recall_context_manager().__enter__()

file_path = str(file.resolve())
try:
reset_top_level_recall_context_manager()
get_top_level_recall_context_manager().__enter__()

file_path = str(file.resolve())

var_context: dict[str, object] = {
"__file__": file_path,
display_decorator_func_name: _display_decorator_function_def,
}

# Execute each top-level node in the AST
for node in tree.body:
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
exec(
compile(ast.Module([node], type_ignores=[]), file_path, "exec"),
var_context,
var_context,
)
else:
exec(
compile(
ast.Interactive([node], type_ignores=[]), file_path, "single"
),
var_context,
var_context,
)

# When we called the function to get the top level recall context manager, we didn't
# store the result in a variable and re-use that variable here. That is intentional,
# because during the evaluation of the app code,
# replace_top_level_recall_context_manager() may have been called, which swaps
# out the context manager, and it's the new one that we need to exit here.
get_top_level_recall_context_manager().__exit__(None, None, None)

# If we're running as an Express app but there's also a top-level item named app
# which is a shiny.App object, the user probably made a mistake.
if "app" in var_context and isinstance(var_context["app"], App):
raise RuntimeError(
"This looks like a Shiny Express app because it imports shiny.express, "
"but it also looks like a Shiny Classic app because it has a variable named "
"`app` which is a shiny.App object. Remove either the shiny.express import, "
"or the app=App()."
)

var_context: dict[str, object] = {
"__file__": file_path,
display_decorator_func_name: _display_decorator_function_def,
}
return ui_result

# Execute each top-level node in the AST
for node in tree.body:
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
exec(
compile(ast.Module([node], type_ignores=[]), file_path, "exec"),
var_context,
var_context,
)
else:
exec(
compile(ast.Interactive([node], type_ignores=[]), file_path, "single"),
var_context,
var_context,
)
except AttributeError as e:
# Need to catch AttributeError and convert to a different type of error, because
# uvicorn specifically catches AttributeErrors and prints an error message that
# is helpful for normal ASGI apps, but misleading in the case of Shiny Express.
raise RuntimeError(e) from e

# When we called the function to get the top level recall context manager, we didn't
# store the result in a variable and re-use that variable here. That is intentional,
# because during the evaluation of the app code,
# replace_top_level_recall_context_manager() may have been called, which swaps
# out the context manager, and it's the new one that we need to exit here.
get_top_level_recall_context_manager().__exit__(None, None, None)

# If we're running as an Express app but there's also a top-level item named app
# which is a shiny.App object, the user probably made a mistake.
if "app" in var_context and isinstance(var_context["app"], App):
raise RuntimeError(
"This looks like a Shiny Express app because it imports shiny.express, "
"but it also looks like a Shiny Classic app because it has a variable named "
"`app` which is a shiny.App object. Remove either the shiny.express import, "
"or the app=App()."
)

return ui_result
finally:
sys.displayhook = prev_displayhook


_top_level_recall_context_manager: RecallContextManager[Tag]
Expand Down

0 comments on commit 6a28a59

Please sign in to comment.