search

amplitude / Amplitude-Node

Server-side Node.js SDK for Amplitude
MIT License
67 stars 20 forks source link

Sending a user_id of a number will 200 but event won't show in Dashboard #63

Open tom-james-watson opened 3 years ago

tom-james-watson commented 3 years ago

If I send a user_id of 123, I get a 200 response, but the event never appears in the web app, although I can see it in the "Successful requests" in "Sources".

Expected Behavior

400 response, as user_id should be a string

haoliu-amp commented 3 years ago

Hello, user_id should be a string type.

tom-james-watson commented 3 years ago

Yeah I understand that, therefore the fact that you can send an integer and not get an error response, and have the event silently disappear, is a bug.

kelvin-lu commented 3 years ago

@tom-james-watson in your 200 response, do you have logs of the other metadata? e.g. how many events were ingested. in 1.0.x and onwards there is an issue where we silently throw away events with neither user nor device id.

when I hit the http api with 123 I get a different response - it errors because it is too short. I've found that creating numeric user ids of length > 5 do get accepted though.

tom-james-watson commented 3 years ago

This was just basic testing in dev, so very limited number of events.

For what it's worth, the offending number was 4 chars, eg 3146. To be clear, this is 3146, not "3146".

On Wed, 18 Nov 2020, 20:50 Kelvin Lu, notifications@github.com wrote:

@tom-james-watson https://github.com/tom-james-watson in your 200 response, do you have logs of the other metadata? e.g. how many events were ingested. in 1.0.x and onwards there is an issue where we silently throw away events with neither user nor device id.

when I hit the http api with 123 I get a different response - it errors because it is too short. I've found that creating numeric user ids of length > 5 do get accepted though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/amplitude/Amplitude-Node/issues/63#issuecomment-729915034, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5MTL2TELLXU3VCFGF2JMDSQQQP3ANCNFSM4T2J3GRA .

Galkon commented 3 years ago

+1 here, we ran into this and were stuck on it for over a day. It's easy to overlook, the request should definitely not 200 when it fails to log the event or track anything. We were trying to track "user_id": 15479253 and that would give us a 200, with eventsIngested = 0 in the callback. We finally spotted that it needed to be a string, and we got a 200 with eventsIngested = 1.

Amplitude should handle invalid data in a better way.

kelvin-lu commented 3 years ago

Hey @Galkon ,

Sorry to hear that. We'll pass this on to our data ingestion team for the response being returned by the HTTP v2 API, and also include some better tooling in this SDK to validate the events better.