-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: add send event method customization #68
Conversation
Sentry issue: SNAPSHOT-WEBHOOK-1 |
But why do we need to add support for the |
event.token = sha256(`${to}${serviceEventsSalt}`); | ||
event.secret = sha256(`${to}${serviceEventsSalt}`); | ||
const headerSecret = sha256(`${to}${process.env.SERVICE_EVENTS_SALT}`); | ||
const url = to.replace('[PROPOSAL-ID]', event.id.split('/')[1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel this URL format is only useful for our case, others may not use it or we need to find a way to send event type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. We can and should add support to inject all the other parameters send in the post body as a query params placeholder. And we should also support head request
@aurelianoB What do you think about this? |
🧿 Current issues / What's wrong ?
All http webhooks events are now sent via a
POST
method, andGET
is not supported.This is the first step to get rid of this hardcoded piece of code: https://github.com/snapshot-labs/snapshot-webhook/blob/master/src/events.ts#L110-L115 , which is raising a lot of errors when sequencer has issues.
Fix #56
💊 Fixes / Solution
Support
GET
,HEAD
and all other methods🚧 Changes
ALTER TABLE subscribers ADD COLUMN method VARCHAR(5) NOT NULL DEFAULT 'POST';
)🛠️ Tests