Skip to content

Conversation

adriandole
Copy link
Contributor

DRIVERS-2421 requests that timeout messages include the topology description and a list of servers for timeout messages.

The C driver already gathers errors from each server (including the URI). This PR adds a topology description to the end of server selection error messages. It is added last so is omitted if there's no space left in mongoc_error_t.

@adriandole adriandole force-pushed the topology-description branch from 5202e26 to 3e5caef Compare February 6, 2024 21:47
@adriandole adriandole force-pushed the topology-description branch from 3e5caef to 6a757da Compare February 6, 2024 22:01
Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good, but I think the new exported function should be removed if it isn't otherwise used.

@kevinAlbs kevinAlbs removed the request for review from vector-of-bool May 1, 2024 12:10
@kevinAlbs kevinAlbs dismissed vector-of-bool’s stale review May 1, 2024 12:10

I approved latest changes. Want to include in upcoming release.

@kevinAlbs kevinAlbs merged commit d50f650 into mongodb:master May 1, 2024
kevinAlbs added a commit to kevinAlbs/mongo-c-driver that referenced this pull request May 1, 2024
@kevinAlbs
Copy link
Collaborator

I merged to attempt to include in the 1.27.0 release. Windows test tasks started to fail. I re-ran a newly failing task on the previous commit. The task on the previous commit passed. So I expect the failure was introduced by those changes. I reverted the commit. I expect a new PR is needed. Sorry for the noise.


done:
if (error) {
_mongoc_error_append (error, topology_type->str);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If error is non-NULL and an error was not set, I expect this is appending to an uninitialized bson_error_t. This may have been the cause of the Windows task failures.

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