Skip to content

fix: return failed ActionResult instead of 500 for typed endpoints (#187) - #256

Merged
RyanTheRobothead merged 7 commits into
unstablefrom
fix/187-failed-action-result-500
Mar 27, 2026
Merged

fix: return failed ActionResult instead of 500 for typed endpoints (#187)#256
RyanTheRobothead merged 7 commits into
unstablefrom
fix/187-failed-action-result-500

Conversation

@RyanTheRobothead

Copy link
Copy Markdown
Member

Summary

  • Wraps json_result_type in Optional[] in create_dynamic_model() so Pydantic accepts None for failed/in-progress actions instead of returning a 500
  • Adds client-side fallback in get_action_result_by_name() to the generic endpoint if the typed endpoint errors
  • Adds regression test that verifies a failed typed action (-> int) returns 200 with status=failed

Closes #187

Test plan

  • New regression test test_failed_typed_action_returns_200 passes
  • All 73 REST node module tests pass
  • All 2148 tests across madsci_common, madsci_node_module, madsci_client pass
  • Ruff lint clean

🤖 Generated with Claude Code

RyanTheRobothead and others added 6 commits March 26, 2026 22:52
Adds :deep='2' and :showLength='true' to the vue-json-pretty component
in the 'Show Copyable Workflow Step' section of NodeModal.vue.

Without depth limiting, actions with deeply nested argument defaults
(like run_assay) create an enormous DOM tree that freezes the browser.

Fixes #157
Replace shared copy ref with per-action Record so toggling the
copyable step for one action doesn't affect other action panels.
Replace manual string concatenation in set_text() with yaml.dump(),
which was already imported but unused. Remove the now-dead cleanArgs()
function and an erroneous stream/consumers import.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d freeze

The root cause of the dashboard freeze when clicking 'Show Copyable
Workflow Step' was @VUE:updated lifecycle hooks on text fields (lines
97 and 127) calling set_text(action), which modifies reactive refs
(json_text, text) mid-render. This triggered a re-render cascade
that caused a stack overflow (RangeError: Maximum call stack size
exceeded) with 8+ second hangs.

Replace @VUE:updated with @update:modelValue so set_text() only runs
when the user actually changes a value, not on every component
re-render. This reduces render time from ~8500ms to ~14ms.

Fixes #157

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ints (#187)

When a typed action (e.g. `-> int`) fails, the action-specific result
endpoint returned a 500 because Pydantic rejected `json_result=None`
against the non-Optional return type. Wrap `json_result_type` in
`Optional[]` in `create_dynamic_model()`, add a client-side fallback
to the generic endpoint, and add a regression test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Mar 27, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/madsci_client/madsci/client/node
  rest_node_client.py 445-456
  src/madsci_common/madsci/common/types
  action_types.py
Project Total  

This report was generated by python-coverage-comment-action

Narrow exception catch from bare `except Exception` to
`requests.HTTPError` with a 5xx status check so non-server errors
are re-raised. Add structured warning log on fallback.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@RyanTheRobothead
RyanTheRobothead merged commit a56cb57 into unstable Mar 27, 2026
4 checks passed
@RyanTheRobothead
RyanTheRobothead deleted the fix/187-failed-action-result-500 branch April 9, 2026 19:47
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.

1 participant