fix(node): Make sure we use same ID for checkIns#8050
Conversation
packages/types/src/checkin.ts
Outdated
| // The distinct slug of the monitor. | ||
| monitorSlug: SerializedCheckIn['monitor_slug']; | ||
| // Check-In ID (unique and client generated). | ||
| checkInId?: SerializedCheckIn['check_in_id']; |
There was a problem hiding this comment.
You could theoretically call captureCheckIn with status "in_progress" but not provide a checkInId. We should couple those types. Not coupling would be very opaque for users why things are not working.
There was a problem hiding this comment.
I think that commit is on the wrong branch
packages/types/src/checkin.ts
Outdated
| // The status of the check-in. | ||
| status: 'ok' | 'error'; | ||
| // Check-In ID (unique and client generated). | ||
| checkInId?: SerializedCheckIn['check_in_id']; |
There was a problem hiding this comment.
Do we want to make the ID required for FinishedCheckIns?
There was a problem hiding this comment.
Honestly, why the hell do we need both an ID and a name??
There was a problem hiding this comment.
Do we want to make the ID required for FinishedCheckIns
It makes the DX worse, so no. This is because technically an ID can be undefined if the client is disabled (so the first captureCheckIn will return undefined in that case)
Honestly, why the hell do we need both an ID and a name??
It's because you can have multiple crons with the same name fire at the same time, the id helps make sure that this works.
There was a problem hiding this comment.
Just to explain the point I was making here because I think you misunderstood me here: For the finished checkins we theoretically don't need to send the id AND the slug since we can already map from id to slug in the product.
I still stand by my point that we should not be required to send both but I'll approve this to unblock so that our current implementation is not busted.
Based on work in #8039, this PR fixes a bug where we were adding a unique ID to every check in, which is incorrect behaviour. Instead we now return a
checkInIdevery time a check in is captured, which can be used to link check ins together.Example: