Update required fields and validation error message #1
Conversation
I also combined formatting changes in this commit. Should have separated them out. Sorry!
- Define the required fields in an array so they can be easily updated. Document all required fields. - Build a descriptive error during request validation so that the response makes clear which fields were required and not provided. - Require timestamp field on all events.
| key: request.body.key, | ||
| value: request.body.value, | ||
| const body = request.body; | ||
| console.log(`Body = ${JSON.stringify(body)}`); |
There was a problem hiding this comment.
This is useful for debugging, but I'd remove this for production, since it will result in a lot of logs with unclear retention policies.
| // Timestamp for when the client generated the event. NOTE: This should be | ||
| // when the event actually occurred on the client and NOT when it was sent | ||
| // to the server. | ||
| 'timestamp', |
There was a problem hiding this comment.
I think it might be nice for the two timestamps to have more "symmetrical" names, e.g., clientTimestamp and serverTimestamp. (I'm also okay with the original field names, sent/received.)
(In general, I want to nudge us to use the server timestamp; the client timestamp should be used primarily for debugging, because it can be way off due to clock skew.)
| uid: "123", | ||
| clientVersion: "v0.1.2", | ||
| timestamp: "2020-04-22T19:09:04.641Z", | ||
| customFieldA: "you can add as many custom fields as you want at the top level", |
There was a problem hiding this comment.
I'd prefer custom fields to be down in value. That way, the top level has a finite and final list of fields, and we know that any additional or metadata fields we might add are out of the client's control.
| } | ||
|
|
||
| // Store data | ||
| store(event) | ||
| .then(() => { | ||
| response.send("OK"); | ||
| response.sendStatus(200); |
There was a problem hiding this comment.
I'm not sure about the default behavior of the 200 response on Firebase, but can you make sure we still get some sort of text acknowledgment at the end of request? It's useful for manual debugging.
No description provided.