Skip to content

Conversation

@jemiahw
Copy link
Collaborator

@jemiahw jemiahw commented Oct 7, 2025

CDCEvent reads the _id field into a string. By default, the Mongo client v2 will not convert the ObjectID to a string, so we get an error at read time. To fix this, we now set BSONOptions on the collection to allow this deserialization. This wasn't caught in the previous tests because the test code explicitly set the _id field to a string when creating test events, so the test events deserialized without error. I've also updated the TestWriteSuccess test to use a Mongo generated _id which reproduces the error.

Alternatively I considered changing CDCEvent.ID to a bson.ObjectID type, but since this struct is used in the REST client library I do not want to leak that Mongo type and dependency into the client. I also considered creating an internal cdcEvent struct identical to CDCEvent but with the bson.ObjectID type, then have the server deserialize into that struct and copy to the CDCEvent struct, but that seemed heavy and unnecessary.

CDCEvent reads the _id field into a string. By default, the Mongo client v2 will not convert the ObjectID to a string, so we get an error at read time. To fix this, we now set BSONOptions on the collection to allow this deserialization. This wasn't caught in the previous tests because the test code explicitly set the _id field to a string when creating test events, so the test events deserialized without error. I've also updated the TestWriteSuccess test to use a Mongo generated _id which reproduces the error.

Alternatively I considered changing CDCEvent.ID to a bson.ObjectID type, but since this struct is used in the REST client library I do not want to leak that Mongo type and dependency into the client. I also considered creating an internal cdcEvent struct identical to CDCEvent but with the bson.ObjectID type, then have the server deserialize into that struct and copy to the CDCEvent struct, but that seemed heavy and unnecessary.
allanliu
allanliu previously approved these changes Oct 8, 2025
@jemiahw jemiahw merged commit 23a1b7d into master Oct 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants