Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

feat: Use new WebhookJobError struct for error reporting #13

Merged
merged 5 commits into from
Dec 19, 2023

Conversation

tomasfarias
Copy link
Contributor

Instead of passing errors just as strings, we should support the app_metrics error schema and use a struct with type, details, and uuid fields; a new WebhookJobError was added to support this schema. Consumer now passes the new struct to fail and retry calls. These methods were already implemented generically over anything that can be serialized to json, so no changes required from that side to support a struct instead of String.

TODO: Should the UUID have any meaning? I've included it mostly to support the app_metrics schema, but I'm generating a new one on every error, so it's not very useful information (at least for now).

@tomasfarias tomasfarias changed the title refactor: Add a metadata field to job queue feat: Use new WebhookJobError struct for error reporting Dec 18, 2023
Copy link
Contributor

@bretthoerner bretthoerner left a comment

Choose a reason for hiding this comment

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

Should the UUID have any meaning?

I couldn't find any meaning in plugin-server, I think new UUID7 is fine/expected.

@tomasfarias tomasfarias changed the base branch from refactor/add-metadata-field to main December 19, 2023 11:12
@tomasfarias tomasfarias changed the base branch from main to refactor/add-metadata-field December 19, 2023 14:12
Base automatically changed from refactor/add-metadata-field to main December 19, 2023 14:18
hook-common/src/webhook.rs Outdated Show resolved Hide resolved
@tomasfarias tomasfarias merged commit 44cdad7 into main Dec 19, 2023
4 checks passed
@tomasfarias tomasfarias deleted the feat/webhook-error branch December 19, 2023 16:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants