Skip to content

Commit 30d6f11

Browse files
committed
on error without returning outputs, fix celery on_error.
1 parent 40130bd commit 30d6f11

File tree

8 files changed

+76
-67
lines changed

8 files changed

+76
-67
lines changed

dash/_callback.py

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ def insert_callback(
233233
long=None,
234234
manager=None,
235235
running=None,
236-
dynamic_creator=False,
236+
dynamic_creator: Optional[bool] = False,
237237
no_output=False,
238238
):
239239
if prevent_initial_call is None:
@@ -279,6 +279,14 @@ def insert_callback(
279279
return callback_id
280280

281281

282+
def _set_side_update(ctx, response) -> bool:
283+
side_update = dict(ctx.updated_props)
284+
if len(side_update) > 0:
285+
response["sideUpdate"] = side_update
286+
return True
287+
return False
288+
289+
282290
# pylint: disable=too-many-branches,too-many-statements
283291
def register_callback(
284292
callback_list, callback_map, config_prevent_initial_callbacks, *_args, **_kwargs
@@ -350,7 +358,7 @@ def add_context(*args, **kwargs):
350358
"callback_context", AttributeDict({"updated_props": {}})
351359
)
352360
callback_manager = long and long.get("manager", app_callback_manager)
353-
app_on_error = kwargs.pop("app_on_error", None)
361+
error_handler = on_error or kwargs.pop("app_on_error", None)
354362

355363
if has_output:
356364
_validate.validate_output_spec(insert_output, output_spec, Output)
@@ -361,7 +369,7 @@ def add_context(*args, **kwargs):
361369
args, inputs_state_indices
362370
)
363371

364-
response = {"multi": True}
372+
response: dict = {"multi": True}
365373
has_update = False
366374

367375
if long is not None:
@@ -413,7 +421,6 @@ def add_context(*args, **kwargs):
413421
triggered_inputs=callback_ctx.triggered_inputs,
414422
ignore_register_page=True,
415423
),
416-
on_error=on_error or app_on_error,
417424
)
418425

419426
data = {
@@ -451,10 +458,19 @@ def add_context(*args, **kwargs):
451458
isinstance(output_value, dict)
452459
and "long_callback_error" in output_value
453460
):
454-
error = output_value.get("long_callback_error")
455-
raise LongCallbackError(
461+
error = output_value.get("long_callback_error", {})
462+
exc = LongCallbackError(
456463
f"An error occurred inside a long callback: {error['msg']}\n{error['tb']}"
457464
)
465+
if error_handler:
466+
error_handler(exc)
467+
output_value = NoUpdate()
468+
# set_props from the error handler uses the original ctx
469+
# instead of manager.get_updated_props since it runs in the
470+
# request process.
471+
has_update = _set_side_update(callback_ctx, response)
472+
else:
473+
raise exc
458474

459475
if job_running and output_value is not callback_manager.UNDEFINED:
460476
# cached results.
@@ -478,16 +494,15 @@ def add_context(*args, **kwargs):
478494
except PreventUpdate as err:
479495
raise err
480496
except Exception as err: # pylint: disable=broad-exception-caught
481-
if on_error:
482-
output_value = on_error(err)
483-
elif app_on_error:
484-
output_value = app_on_error(err)
497+
if error_handler:
498+
error_handler(err)
499+
if not multi:
500+
output_value = NoUpdate()
501+
else:
502+
output_value = [NoUpdate for _ in output_spec]
485503
else:
486504
raise err
487505

488-
if NoUpdate.is_no_update(output_value):
489-
raise PreventUpdate
490-
491506
component_ids = collections.defaultdict(dict)
492507

493508
if has_output:
@@ -508,12 +523,12 @@ def add_context(*args, **kwargs):
508523
)
509524

510525
for val, spec in zip(flat_output_values, output_spec):
511-
if isinstance(val, NoUpdate):
526+
if NoUpdate.is_no_update(val):
512527
continue
513528
for vali, speci in (
514529
zip(val, spec) if isinstance(spec, list) else [[val, spec]]
515530
):
516-
if not isinstance(vali, NoUpdate):
531+
if not NoUpdate.is_no_update(vali):
517532
has_update = True
518533
id_str = stringify_id(speci["id"])
519534
prop = clean_property_name(speci["property"])
@@ -527,10 +542,7 @@ def add_context(*args, **kwargs):
527542
flat_output_values = []
528543

529544
if not long:
530-
side_update = dict(callback_ctx.updated_props)
531-
if len(side_update) > 0:
532-
has_update = True
533-
response["sideUpdate"] = side_update
545+
has_update = _set_side_update(callback_ctx, response) or has_update
534546

535547
if not has_update:
536548
raise PreventUpdate

dash/long_callback/managers/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def job_running(self, job):
3939
def make_job_fn(self, fn, progress, key=None):
4040
raise NotImplementedError
4141

42-
def call_job_fn(self, key, job_fn, args, context, on_error=None):
42+
def call_job_fn(self, key, job_fn, args, context):
4343
raise NotImplementedError
4444

4545
def get_progress(self, key):

dash/long_callback/managers/celery_manager.py

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ def get_task(self, job):
8989
def clear_cache_entry(self, key):
9090
self.handle.backend.delete(key)
9191

92-
def call_job_fn(self, key, job_fn, args, context, on_error=None):
93-
task = job_fn.delay(key, self._make_progress_key(key), args, context, on_error)
92+
def call_job_fn(self, key, job_fn, args, context):
93+
task = job_fn.delay(key, self._make_progress_key(key), args, context)
9494
return task.task_id
9595

9696
def get_progress(self, key):
@@ -139,9 +139,7 @@ def _make_job_fn(fn, celery_app, progress, key):
139139
cache = celery_app.backend
140140

141141
@celery_app.task(name=f"long_callback_{key}")
142-
def job_fn(
143-
result_key, progress_key, user_callback_args, context=None, on_error=None
144-
):
142+
def job_fn(result_key, progress_key, user_callback_args, context=None):
145143
def _set_progress(progress_value):
146144
if not isinstance(progress_value, (list, tuple)):
147145
progress_value = [progress_value]
@@ -181,21 +179,18 @@ def run():
181179
),
182180
)
183181
except Exception as err: # pylint: disable=broad-except
184-
if on_error:
185-
user_callback_output = on_error(err)
186-
else:
187-
errored = True
188-
cache.set(
189-
result_key,
190-
json.dumps(
191-
{
192-
"long_callback_error": {
193-
"msg": str(err),
194-
"tb": traceback.format_exc(),
195-
}
196-
},
197-
),
198-
)
182+
errored = True
183+
cache.set(
184+
result_key,
185+
json.dumps(
186+
{
187+
"long_callback_error": {
188+
"msg": str(err),
189+
"tb": traceback.format_exc(),
190+
}
191+
},
192+
),
193+
)
199194

200195
if not errored:
201196
cache.set(

dash/long_callback/managers/diskcache_manager.py

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,14 @@ def clear_cache_entry(self, key):
115115
self.handle.delete(key)
116116

117117
# noinspection PyUnresolvedReferences
118-
def call_job_fn(self, key, job_fn, args, context, on_error=None):
118+
def call_job_fn(self, key, job_fn, args, context):
119119
# pylint: disable-next=import-outside-toplevel,no-name-in-module,import-error
120120
from multiprocess import Process
121121

122122
# pylint: disable-next=not-callable
123123
proc = Process(
124124
target=job_fn,
125-
args=(key, self._make_progress_key(key), args, context, on_error),
125+
args=(key, self._make_progress_key(key), args, context),
126126
)
127127
proc.start()
128128
return proc.pid
@@ -169,7 +169,7 @@ def get_updated_props(self, key):
169169

170170

171171
def _make_job_fn(fn, cache, progress):
172-
def job_fn(result_key, progress_key, user_callback_args, context, on_error):
172+
def job_fn(result_key, progress_key, user_callback_args, context):
173173
def _set_progress(progress_value):
174174
if not isinstance(progress_value, (list, tuple)):
175175
progress_value = [progress_value]
@@ -200,19 +200,16 @@ def run():
200200
errored = True
201201
cache.set(result_key, {"_dash_no_update": "_dash_no_update"})
202202
except Exception as err: # pylint: disable=broad-except
203-
if on_error:
204-
user_callback_output = on_error(err)
205-
else:
206-
errored = True
207-
cache.set(
208-
result_key,
209-
{
210-
"long_callback_error": {
211-
"msg": str(err),
212-
"tb": traceback.format_exc(),
213-
}
214-
},
215-
)
203+
errored = True
204+
cache.set(
205+
result_key,
206+
{
207+
"long_callback_error": {
208+
"msg": str(err),
209+
"tb": traceback.format_exc(),
210+
}
211+
},
212+
)
216213

217214
if not errored:
218215
cache.set(result_key, user_callback_output)

tests/integration/callbacks/test_callback_error.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
from dash import Dash, html, Input, Output
1+
from dash import Dash, html, Input, Output, set_props
22

33

44
def test_cber001_error_handler(dash_duo):
55
def global_callback_error_handler(err):
6-
return f"global: {err}"
6+
set_props("output-global", {"children": f"global: {err}"})
77

88
app = Dash(on_error=global_callback_error_handler)
99

@@ -15,7 +15,7 @@ def global_callback_error_handler(err):
1515
]
1616

1717
def on_callback_error(err):
18-
return f"callback: {err}"
18+
set_props("output", {"children": f"callback: {err}"})
1919

2020
@app.callback(
2121
Output("output", "children"),

tests/integration/long_callback/app_bg_on_error.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
from dash import Dash, Input, Output, html
1+
from dash import Dash, Input, Output, html, set_props
22
from tests.integration.long_callback.utils import get_long_callback_manager
33

44
long_callback_manager = get_long_callback_manager()
55
handle = long_callback_manager.handle
66

77

88
def global_error_handler(err):
9-
10-
return f"global: {err}"
9+
set_props("global-output", {"children": f"global: {err}"})
1110

1211

1312
app = Dash(
@@ -23,7 +22,7 @@ def global_error_handler(err):
2322

2423

2524
def callback_on_error(err):
26-
return f"callback: {err}"
25+
set_props("cb-output", {"children": f"callback: {err}"})
2726

2827

2928
@app.callback(

tests/integration/long_callback/test_basic_long_callback018.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ def test_lcbc018_background_callback_on_error(dash_duo, manager):
77

88
dash_duo.find_element("#start-cb-onerror").click()
99

10-
dash_duo.wait_for_text_to_equal("#cb-output", "callback: callback error")
10+
dash_duo.wait_for_contains_text("#cb-output", "callback error")
1111

1212
dash_duo.find_element("#start-global-onerror").click()
13-
dash_duo.wait_for_text_to_equal("#global-output", "global: global error")
13+
dash_duo.wait_for_contains_text("#global-output", "global error")

tests/integration/long_callback/utils.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,14 @@ def __init__(self, cache=None, cache_by=None, expire=None):
1818
super().__init__(cache=cache, cache_by=cache_by, expire=expire)
1919
self.running_jobs = []
2020

21-
def call_job_fn(self, key, job_fn, args, context, on_error=None):
22-
pid = super().call_job_fn(key, job_fn, args, context, on_error=on_error)
21+
def call_job_fn(
22+
self,
23+
key,
24+
job_fn,
25+
args,
26+
context,
27+
):
28+
pid = super().call_job_fn(key, job_fn, args, context)
2329
self.running_jobs.append(pid)
2430
return pid
2531

0 commit comments

Comments
 (0)