Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose host address when timeout exceeded #1943

Closed
wants to merge 4 commits into from
Closed

Expose host address when timeout exceeded #1943

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5f8a7c
Expose host address when timeout exceeded
laskoviymishka Mar 13, 2024
7309f5c
maybe this would work better?
laskoviymishka Mar 16, 2024
5d03cb9
okay this should be more correct.
laskoviymishka Mar 17, 2024
7d6cfec
nilify result error
laskoviymishka Mar 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pgconn/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func normalizeTimeoutError(ctx context.Context, err error) error {
// Since the timeout was caused by a context cancellation, the actual error is context.Canceled not the timeout error.
return context.Canceled
} else if ctx.Err() == context.DeadlineExceeded {
return &errTimeout{err: ctx.Err()}
return &errTimeout{err: fmt.Errorf("%s: %w", netErr.Error(), ctx.Err())}
} else {
return &errTimeout{err: netErr}
}
Expand Down
4 changes: 4 additions & 0 deletions pgconn/pgconn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ func ConnectConfig(octx context.Context, config *Config) (pgConn *PgConn, err er
if _, ok := cerr.err.(*NotPreferredError); ok {
fallbackConfig = fc
}
if _, ok := cerr.err.(*errTimeout); ok {
// once we reach timeout it's useless to check other fallbacks
break
}
Copy link
Owner

Choose a reason for hiding this comment

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

What I'm saying is it's not always useless to check other fallbacks. errTimeout could come from a TCP timeout. If the context has not expired the next fallback should be tried.

Copy link
Contributor Author

@laskoviymishka laskoviymishka Mar 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Got it.
I made one more attempt :D
So now I do a bit different thing, I keep track on fallback error and override result error only if needed.
For a deadline errors we should keep only first deadline error in result, every next one is false-positive errors (since we not actually do anything).

}
}

Expand Down