Skip to content

feat(taskworker) Validate JSON compatibity of task parameters on direct calls #92420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 30, 2025

Conversation

markstory
Copy link
Member

We often write test where task logic is tested via direct function calls, and assertions for scheduled calls done with mocks. Because of this we can't currently catch JSON incompatibilities in task parameter.

With these changes during tests we will validate that direct function calls also have JSON compatible parameters which should reduce surprises in production.

markstory added 3 commits May 28, 2025 13:00
We often test tasks by calling the functions directly. We should
validate those call sites to ensure that they are not violating JSON
encodable parameters.

Refs #91927
@markstory markstory requested a review from a team May 28, 2025 17:53
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 28, 2025
Copy link

codecov bot commented May 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #92420       +/-   ##
===========================================
+ Coverage   46.23%   87.97%   +41.73%     
===========================================
  Files       10216    10223        +7     
  Lines      585990   586078       +88     
  Branches    22798    22669      -129     
===========================================
+ Hits       270930   515581   +244651     
+ Misses     314630    70069   -244561     
+ Partials      430      428        -2     

@markstory markstory marked this pull request as ready for review May 30, 2025 14:21
@markstory markstory requested review from a team as code owners May 30, 2025 14:21
@markstory markstory merged commit 906447b into master May 30, 2025
61 checks passed
@markstory markstory deleted the feat-validate-parameters branch May 30, 2025 15:14
Copy link

sentry-io bot commented May 30, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ApiError: {"message":"404 File Not Found"} sentry.tasks.process_commit_context View Issue
  • ‼️ ApiError: { sentry.tasks.process_commit_context View Issue
  • ‼️ ReadTimeout: SafeHTTPSConnectionPool(host='app.komodor.com', port=443): Read timed out. (read timeout=2.0) sentry.sentry_apps.tasks.sentry_apps.send_resou... View Issue
  • ‼️ SentryAppSentryError: event_not_in_servicehook sentry.sentry_apps.tasks.sentry_apps.workflow_n... View Issue
  • ‼️ ReadTimeout: SafeHTTPSConnectionPool(host='app.komodor.com', port=443): Read timed out. (read timeout=2.0) sentry.sentry_apps.tasks.sentry_apps.send_resou... View Issue

Did you find this useful? React with a 👍 or 👎

andrewshie-sentry pushed a commit that referenced this pull request Jun 2, 2025
…ct calls (#92420)

We often write test where task logic is tested via direct function
calls, and assertions for scheduled calls done with mocks. Because of
this we can't currently catch JSON incompatibilities in task parameter.

With these changes during tests we will validate that direct function
calls also have JSON compatible parameters which should reduce surprises
in production.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants