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.

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

Is there anyway to prevent reconnection attempt behavior in Credentials? #2863

Closed
iwasaki-kenta opened this issue Jun 11, 2019 · 6 comments
Closed

Comments

@iwasaki-kenta
Copy link

I defined a set of custom credentials, which implement ClientHandshake and ServerHandshake methods. It appears that the default behavior is that if handshaking fails, a reconnection will be attempted.

However, say for example that the handshaking fails as a result of the server claiming that I have provided invalid credentials, or that I have provided credentials that were valid but are unauthorized. How can I forcefully stop retriggering a reconnection to the server in this case?

@lyuxuan
Copy link
Contributor

lyuxuan commented Jun 11, 2019

As indicated here, if the error returned by ClientHandshake implements Temporary() and returns false, then when you Dial with DialOption WithBlock and FailOnNonTempDialError , it will fail for the case you described and not triggering reconnection.

Some relevant context here.

@iwasaki-kenta
Copy link
Author

As indicated here, if the error returned by ClientHandshake implements Temporary() and returns false, then when you Dial with DialOption WithBlock and FailOnNonTempDialError , it will fail for the case you described and not triggering reconnection.

Some relevant context here.

What exactly is a wrapper error under this definition?

	// If the returned error is a wrapper error, implementations should make sure that
	// the error implements Temporary() to have the correct retry behaviors.

The option FailOnNonTempDialError appears to be what I need in this case. Is there any reason why gRPC does not choose to have it enabled by default?

@lyuxuan
Copy link
Contributor

lyuxuan commented Jun 11, 2019

Below is an example of wrapper error.

type errorWrapper struct {
	error // embedding the original error
}

func (e *errorWrapper) Temporary() bool {
    return false
}

In your case, your ClientHandshake implementation knows whether the error should be retried or not, so you can return the error wrapper which wraps around the original error as necessary to force no retry.

For grpc, an attempt to establish a connection may be impacted by many different things (e.g. interrupt in network, server not up yet, etc.), and many of those are temporary and can be resolved by retrying. Very few situations like what you have mentioned is hard failure and retry won't help. Therefore, by default, FailOnNonTempDialError is not on.

@iwasaki-kenta
Copy link
Author

iwasaki-kenta commented Jun 14, 2019

Below is an example of wrapper error.

type errorWrapper struct {
	error // embedding the original error
}

func (e *errorWrapper) Temporary() bool {
    return false
}

In your case, your ClientHandshake implementation knows whether the error should be retried or not, so you can return the error wrapper which wraps around the original error as necessary to force no retry.

For grpc, an attempt to establish a connection may be impacted by many different things (e.g. interrupt in network, server not up yet, etc.), and many of those are temporary and can be resolved by retrying. Very few situations like what you have mentioned is hard failure and retry won't help. Therefore, by default, FailOnNonTempDialError is not on.

Gotcha, one last question then. Using WithBlock(), I noticed that the error returned does not include any information as to why a call to Dial() fails. Removing WithBlock() properly yields an error that describes why the custom handshaking protocol I implemented failed (gRPC propagates upwards my own custom returned errors).

Would this be expected behavior?

@menghanl
Copy link
Contributor

What error did you get from Dial without WithBlock()? Did you also set FailOnNonTempDialError()?

@dfawley
Copy link
Member

dfawley commented Jun 27, 2019

Ping @iwasaki-kenta - do you have any additional information on this?

@dfawley dfawley closed this as completed Jul 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants