ankane / ahoy

Simple, powerful, first-party analytics for Rails
MIT License
4.24k stars 377 forks source link

API POST /ahoy/events returns 200 OK even when event creation failed #472

Closed Startouf closed 3 years ago

Startouf commented 3 years ago

The ahoy endpoint returns a 200 even when the event creation is unsuccessful.

Here is an example of what may happen

2021-01-15T14:17:13+00:00 Processing by Ahoy::EventsController#create as */*
2021-01-15T14:17:13+00:00   Parameters: {"events"=>[{"id"=>"7a58bcd3-bfce-44d6-997a-2bb2ddd3d6f1", "name"=>"Filled company contact email", "properties"=>{"email"=>"test@example.com", "utm"=>{}}, "time"=>1610720232.461}], "visit_token"=>"[FILTERED]", "visitor_token"=>"[FILTERED]", "event"=>{}}
2021-01-15T14:17:13+00:00 Can't verify CSRF token authenticity.
2021-01-15T14:17:13+00:00 [ahoy] Missing required parameters
2021-01-15T14:17:13+00:00 [active_model_serializers] Rendered ActiveModel::Serializer::Null with Hash (0.17ms)
2021-01-15T14:17:13+00:00 Completed 200 OK in 12ms (Views: 10.1ms | MongoDB: 0.0ms | Elasticsearch: 0.0ms | Allocations: 731)

(and nothing is saved in the database)


EDIT : the underlying problem was that the ahoy cookie domain was not set correctly and the JS script would send the events with null visitor + visit token, which caused the "missing required parameter" error in the log (and a 200 status code)

It had appeared as [FILTERED] in my request because I had added some backtrace filters for *_token 😅 but the values were null indeed

{ 
  events: [{id: "5f9afd9d-2ef6-4234-a164-2d0e3518f0f0", name: …}]
  visit_token: null
  visitor_token: null
}
ankane commented 3 years ago

Hey @Startouf, there seems to be a lot here. Can you split it into multiple issues?

Startouf commented 3 years ago

@ankane I have edited the main post to clean up, it turns out the underlying problem was much simpler than I had imagined.

Regarding the other issue I had mentioned... well let's just drop it since I'm no longer concerned by the problem ^^"

ankane commented 3 years ago

Hey @Startouf, I updated the API to return 400 when missing required parameters in Ahoy 4.0 (just released). Thanks for the suggestion.