-
Notifications
You must be signed in to change notification settings - Fork 25
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
Upgrade Sendgrid web hook code to new API #6
Comments
Looks like the post describing the changes are here: http://sendgrid.com/blog/event-webhook-update-batched-vs-unbatched-events/ @adooylabs, want to take a stab and forking this and making a pull request to lee? |
+1 for an update that would support the new format. I was sure 0.2 already supports the new format, since it says in the changelog "0.2 - Supports version 3 of the Sendgrid webhook released on September 6th, 2013, which provides the proper headers and JSON post body without hacks or middleware. If upgrading to this version, please make sure to update the Webhooks settings in your SendGrid App to use V3 of their API" But I guess that's a different change from the one announced in February by SendGrid |
@leejarvis any plans to implement these changes? Unfortunately currently this excellent gem is unusable. |
@mikemarsian pull requests are welcome! ;) |
@mikemarsian Sorry but I don't currently have the time to invest in this. I don't use SendGrid or this gem anymore. I think it would be great for gridhook to support the new API functionality which is why I am calling on people like yourself who use and care about this, to help me maintain and add support for the changing API at SendGrid. As Scott suggests, we're open to pull requests for this functionality, and I'm happy to add commit bit for more contributors so this library remains useable and useful. |
@mikemarsian @adooylabs I just dug into this a bit and as far as I can tell the API changes announced in February don't break this gem. It sounds like they just stopped sending unbatched events, but this gem supports the newer batched format. I just tried a test event notification (https://sendgrid.com/app/appSettings/type/eventnotify/id/15) against the endpoint provided by the gem and sengrid sent over a batch of events which rails identifies as json and passes into the app in the "_json" parameter. This in turn gets handled by this line: https://github.com/leejarvis/gridhook/blob/master/lib/gridhook/event.rb#L14 Were there other changes made to the webhook API that I'm not seeing? |
Hi @stereoscott @adooylabs @leejarvis , Here are the RequestBin's of both posts: HTTP post using SendGrid test tool (gridhook returns 200 ok, but doesn't fire up any events in the app): HTTP post using Postman with the same data (gridhook fires up events and everything works just fine): I have tried adding newlines to postman's data (to be similar to Sendgrid's format), and it still works. Any idea what's going on? |
At a quick glance it looks like the request content-type differs ( |
@mikemarsian if you don't mind, give this branch a go: https://github.com/quikly/gridhook/tree/accept-plain-text That version will attempt parse the post body as JSON even if the json content-type header is not set. |
I'm wondering why when sendgrid notifies my app it uses the proper content type, but @mikemarsian is seeing something different. I'm still using this gem in production and the sendgrid notifications continue to work properly. And when sendgrid says to test out your local implementation using curl to run:
...which seems to imply they send the correct content-type header (see: https://sendgrid.com/docs/API_Reference/Webhooks/event.html) |
Sending post with curl with SenGrid's provided example (that has application/json content type) works just fine with my gridhook. So it must be something else. |
@stereoscott I'm using the branch you suggested and it works perfectly with the current SendGrid API. Is there any chance accept those changes as a pull request? Anything I could do to help this happen? I think it's best for all if we keep it that way. |
Last feb sendgrid announced that they are changing their event notification API. Any plans with regards to this?
The text was updated successfully, but these errors were encountered: