Skip to content

Conversation

@qingyang-hu
Copy link
Collaborator

@qingyang-hu qingyang-hu commented Mar 3, 2025

GODRIVER-2721

Summary

This PR draft is not meant to be merged.

This PR and #1984 replace error assertions (except those in replaceErrors() in "mongo\errors.go") with errors.Is() and errors.As().

However, the error assertion checks the immediate error type. Instead, errors.Is()/errors.As() trace the error tree. The difference changes the current error handling logic so they are not proper. Moreover, any further replacements in replaceErrors() will cause regression failures because of the logic change. The difference changes the error handling logic so any further replacements in replaceErrors() will cause regression failures. replaceErrors() will be replaced in another PR.

In conclusion, we should not replace error assertions with errors.Is()/errors.As() in the current code base.

We can consider closing GODRIVER-2721 and GODRIVER-2975, or making GODRIVER-2721 an epic for tickets such as GODRIVER-1854 and GODRIVER-2704.

Background & Motivation

@mongodb-drivers-pr-bot
Copy link
Contributor

API Change Report

No changes found!

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the review-priority-low Low Priority PR for Review: within 3 business days label Mar 4, 2025
@matthewdale matthewdale self-requested a review March 4, 2025 19:58
matthewdale
matthewdale previously approved these changes Mar 17, 2025
Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

Comment on lines 54 to 57
func isNamespaceNotFoundError(err error) bool {
if de, ok := err.(driver.Error); ok {
var de driver.Error
if errors.As(err, &de) {
return de.Code == 26
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: Can we use the NamespaceNotFound method here?
Optional: If we use NamespaceNotFound, can we remove this function?

@qingyang-hu qingyang-hu merged commit 9443c03 into mongodb:master Mar 21, 2025
32 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-priority-low Low Priority PR for Review: within 3 business days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants