Skip to content

fix: avoid /wait webhook crash on InlineObject workflow results#2245

Merged
daryllimyt merged 8 commits intomainfrom
fix/webhook-wait-regression
Mar 3, 2026
Merged

fix: avoid /wait webhook crash on InlineObject workflow results#2245
daryllimyt merged 8 commits intomainfrom
fix/webhook-wait-regression

Conversation

@daryllimyt
Copy link
Contributor

@daryllimyt daryllimyt commented Mar 3, 2026

Summary

  • fix /webhooks/{workflow_id}/{secret}/wait regression where debug logging raised TypeError when workflow results included non-JSON-serializable objects (for example InlineObject)
  • replace json.dumps(result) in _dispatch_workflow with structured logger field output
  • add _dispatch_workflow regression test to assert dispatch returns successfully when workflow result contains an InlineObject
  • add router-level /wait handler test to assert incoming_webhook_wait returns response["result"] unchanged and forwards TriggerType.WEBHOOK

Root cause

WorkflowExecutionsService._dispatch_workflow was logging workflow results with:

json.dumps(result, indent=2)

When Temporal returned a result containing InlineObject, json.dumps raised TypeError: Object of type InlineObject is not JSON serializable, which bubbled up and caused the webhook /wait request to return HTTP 500.

Example

~/dev/org/trees/tracecat/fix-webhook-wait-regression fix/webhook-wait-regression *162                                                           26s 15:12:50
> curl -X POST "http://localhost:380/api/webhooks/wf_702uqaTiUW94FX1QVZ5Qau/503bbd3fc9052d41de93b186c5477c6a7a4d3904ca072deea55059de4630e1b6/wait"
{"kind":"value","value":1}%

Testing

  • PG_PORT=5732 TEMPORAL_PORT=7533 MINIO_PORT=9300 REDIS_PORT=6679 TRACECAT__SERVICE_KEY=test-service-key uv run pytest tests/unit/test_webhook_execution_path.py -x
  • uv run ruff check tracecat/workflow/executions/service.py tests/unit/test_webhook_execution_path.py
  • uv run pyright tracecat/workflow/executions/service.py tests/unit/test_webhook_execution_path.py

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tracecat/webhooks/router.py">

<violation number="1" location="tracecat/webhooks/router.py:197">
P2: `/wait` now always wraps non-stored results in a `{kind: "value"}` envelope, changing the response shape for plain values despite the docstring saying only StoredObject variants should be normalized. This can break existing clients expecting raw result values.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@daryllimyt daryllimyt requested a review from jordan-umusu March 3, 2026 18:25
Copy link
Collaborator

@jordan-umusu jordan-umusu left a comment

Choose a reason for hiding this comment

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

LGTM

@daryllimyt
Copy link
Contributor Author

@jordan-umusu

Follow-up on the /wait fix: I removed the backend-coupled collection path (get_object_storage().retrieve(collection)) and made collection materialization backend-agnostic.

What changed:

  • CollectionObject is now materialized from collection pages/chunks directly (get_collection_page) instead of delegating to the active object-storage backend.
  • Nested stored objects are resolved explicitly by type:
    • InlineObject -> returned as raw value in the kind: "value" envelope
    • ExternalObject -> downloaded + integrity checked + deserialized
    • CollectionObject -> recursively materialized
  • /wait response shape is a tagged union (value, download_file, download_export) so clients can branch deterministically.

Why this fixes the regression:

  • With TRACECAT__RESULT_EXTERNALIZATION_ENABLED=false, API may still read collection-backed results produced elsewhere. Because /wait no longer depends on backend retrieve(CollectionObject), it won’t fail on InlineObjectStorage and still returns a valid download/export response.

Coverage added:

  • tests/unit/test_webhook_execution_path.py::TestWebhookRouterExecutionPath::test_wait_webhook_materializes_collection_without_storage_backend

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tracecat/webhooks/router.py">

<violation number="1" location="tracecat/webhooks/router.py:179">
P2: CollectionObject.index is ignored when materializing values for /wait, so a CollectionObject pointing at a single item will return the entire collection. This can expose extra data and create large responses. Handle collection.index by fetching only the indexed item (e.g., get_collection_item) before materializing.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@daryllimyt daryllimyt merged commit cefc6b7 into main Mar 3, 2026
20 checks passed
@daryllimyt daryllimyt deleted the fix/webhook-wait-regression branch March 3, 2026 21:43
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.

2 participants