From 04c0ba909d79874baa807a6e91bea74ed40b4f03 Mon Sep 17 00:00:00 2001 From: ulas Date: Sat, 1 Apr 2023 13:15:00 +0300 Subject: [PATCH 1/9] re-commit documentation --- Documentation/error-handling.md | 175 ++++++++++++++++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100644 Documentation/error-handling.md diff --git a/Documentation/error-handling.md b/Documentation/error-handling.md new file mode 100644 index 000000000000..5c48f83c9dc0 --- /dev/null +++ b/Documentation/error-handling.md @@ -0,0 +1,175 @@ +## Dialing In gRPC + +`grpc.Dial` is a function in the gRPC library that creates a virtual connection +from the gRPC client to the gRPC server. It takes a server URI (which can +represent the name of a logical backend service and could resolve to multiple +actual addresses) and a list of options, and returns a `ClientConn` object that +represents the connection to the server. The `ClientConn` contains one or more +actual connections to real server backends and attempts to keep these +connections healthy by automatically reconnecting to them (with an exponential +backoff) when they break. + +The `grpc.Dial` function can also be configured with various options to +customize the behavior of the client connection. For example, developers could +use options such a `WithTransportCredentials()` to configure the transport +credentials to use. + +While `grpc.Dial` is commonly referred to as a "dialing" function, it doesn't +actually perform the low-level network dialing operation like a typical TCP +dialing function would. Instead, it creates a virtual connection from the gRPC +client to the gRPC server. + +`grpc.Dial` does initiate the process of connecting to the server, but it uses +the ClientConn object to manage and maintain that connection over time. This is +why errors encountered during the initial connection are no different from those +that occur later on, and why it's important to handle errors from RPCs rather +than relying on options like `FailOnNonTempDialError`, `WithBlock`, and +`WithReturnConnectionError`. + +### Using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` + +The gRPC API provides several options that can be used to configure the behavior +of dialing and connecting to a gRPC server. Some of these options, such as +`FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError`, rely on +failures at dial time. However, we strongly discourage developers from using +these options, as they can introduce race conditions and result in unreliable +and difficult-to-debug code. + +### Why we discourage using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` + +When a client attempts to connect to a gRPC server, it can encounter a variety +of errors, including network connectivity issues, server-side errors, and +incorrect usage of the gRPC API. The options `FailOnNonTempDialError`, +`WithBlock`, and `WithReturnConnectionError` are designed to handle some of +these errors, but they do so by relying on failures at dial time. This means +that they may not provide reliable or accurate information about the status of +the connection. + +For example, if a client uses `WithBlock` to wait for a connection to be +established, it may end up waiting indefinitely if the server is not responding. +Similarly, if a client uses `WithReturnConnectionError` to return a connection +error if dialing fails, it may miss opportunities to recover from transient +network issues that are resolved shortly after the initial dial attempt. + +## Best practices for error handling in gRPC + +Instead of relying on failures at dial time, we strongly encourage developers to +rely on errors from RPCs. When a client makes an RPC, it can receive an error +response from the server. These errors can provide valuable information about +what went wrong, including information about network issues, server-side errors, +and incorrect usage of the gRPC API. + +By handling errors from RPCs correctly, developers can write more reliable and +robust gRPC applications. Here are some best practices for error handling in +gRPC: + +- Always check for error responses from RPCs and handle them appropriately. +- Use the `status` field of the error response to determine the type of error that + occurred. +- When retrying failed RPCs, use a backoff strategy to avoid + overwhelming the server or exacerbating network issues. +- Avoid using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError`, + as these options can introduce race conditions and result in unreliable and + difficult-to-debug code. + +## Example: Handling errors from an RPC + +The following code snippet demonstrates how to handle errors from an RPC in +gRPC: + +```go +ctx, cancel := context.WithTimeout(context.Background(), time.Second) +defer cancel() + +res, err := client.MyRPC(ctx, &MyRequest{}) +if err != nil { + // Handle the error appropriately, + // log it & return an error to the caller, etc. + log.Printf("Error calling MyRPC: %v", err) + return nil, err +} + +// Use the response as appropriate +log.Printf("MyRPC response: %v", res) + +``` + +To determine the type of error that occurred, you can use the status field of +the error response: + + +```go +resp, err := client.MakeRPC(context.Background(), request) +if err != nil { + status, ok := status.FromError(err) + if ok { + // Handle the error based on its status code + if status.Code() == codes.NotFound { + log.Println("Requested resource not found") + } else { + log.Printf("RPC error: %v", status.Message()) + } + } else { + //Handle non-RPC errors + log.Printf("Non-RPC error: %v", err) + } + return +} + +// Use the response as needed +log.Printf("Response received: %v", resp) +``` + +## Example: Using a backoff strategy + + +When retrying failed RPCs, use a backoff strategy to avoid overwhelming the +server or exacerbating network issues: + + +```go +var res *MyResponse +var err error + +// Retry the RPC call a maximum number of times +for i := 0; i < maxRetries; i++ { + // Create a context with a timeout of one second + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + // Make the RPC call + res, err = client.MyRPC(ctx, &MyRequest{}) + + // Check if the RPC call was successful + if err == nil { + // The RPC was successful, so break out of the loop + break + } + + // The RPC failed, so wait for a backoff period before retrying + backoff := time.Duration(i) * time.Second + log.Printf("Error calling MyRPC: %v; retrying in %v", err, backoff) + time.Sleep(backoff) +} + +// Check if the RPC call was successful after all retries +if err != nil { + // All retries failed, so handle the error appropriately + log.Printf("Error calling MyRPC: %v", err) + return nil, err +} + +// Use the response as appropriate +log.Printf("MyRPC response: %v", res) + +``` + + +## Conclusion + +The `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` +options are designed to handle errors at dial time, but they can introduce race +conditions and result in unreliable and difficult-to-debug code. Instead of +relying on these options, we strongly encourage developers to rely on errors +from RPCs for error handling. By following best practices for error handling in +gRPC, developers can write more reliable and robust gRPC applications. From 97235f7113b75153b0472b6f44a9edc83cbf9bbc Mon Sep 17 00:00:00 2001 From: ulas Date: Wed, 5 Apr 2023 01:37:51 +0300 Subject: [PATCH 2/9] fixes according the reviews --- .../{error-handling.md => anti-patterns.md} | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) rename Documentation/{error-handling.md => anti-patterns.md} (84%) diff --git a/Documentation/error-handling.md b/Documentation/anti-patterns.md similarity index 84% rename from Documentation/error-handling.md rename to Documentation/anti-patterns.md index 5c48f83c9dc0..b2402316d404 100644 --- a/Documentation/error-handling.md +++ b/Documentation/anti-patterns.md @@ -1,30 +1,35 @@ -## Dialing In gRPC +## Anti-Patterns +### Dialing in gRPC `grpc.Dial` is a function in the gRPC library that creates a virtual connection -from the gRPC client to the gRPC server. It takes a server URI (which can +from the gRPC client to the gRPC server. It takes a target URI (which can represent the name of a logical backend service and could resolve to multiple actual addresses) and a list of options, and returns a `ClientConn` object that represents the connection to the server. The `ClientConn` contains one or more actual connections to real server backends and attempts to keep these -connections healthy by automatically reconnecting to them (with an exponential -backoff) when they break. +connections healthy by automatically reconnecting to them when they break. -The `grpc.Dial` function can also be configured with various options to +The `Dial` function can also be configured with various options to customize the behavior of the client connection. For example, developers could -use options such a `WithTransportCredentials()` to configure the transport +use options such a `WithTransportCredentials` to configure the transport credentials to use. -While `grpc.Dial` is commonly referred to as a "dialing" function, it doesn't -actually perform the low-level network dialing operation like a typical TCP -dialing function would. Instead, it creates a virtual connection from the gRPC +While `Dial` is commonly referred to as a "dialing" function, it doesn't +actually perform the low-level network dialing operation like `net.Dial` would. +Instead, it creates a virtual connection from the gRPC client to the gRPC server. -`grpc.Dial` does initiate the process of connecting to the server, but it uses +`Dial` does initiate the process of connecting to the server, but it uses the ClientConn object to manage and maintain that connection over time. This is why errors encountered during the initial connection are no different from those that occur later on, and why it's important to handle errors from RPCs rather than relying on options like `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError`. +In fact, `Dial` does not always establish a connection to servers by default. The connection +behavior is determined by the load balancing policy being used. For instance, an "active" +load balancing policy such as Round Robin attempts to maintain a constant connection, while the default "pick first" +policy delays connection until an RPC is executed. To explicitly initiate a connection, you can employ +the WithBlock option. ### Using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` @@ -72,7 +77,7 @@ gRPC: as these options can introduce race conditions and result in unreliable and difficult-to-debug code. -## Example: Handling errors from an RPC +### Example: Handling errors from an RPC The following code snippet demonstrates how to handle errors from an RPC in gRPC: @@ -120,7 +125,7 @@ if err != nil { log.Printf("Response received: %v", resp) ``` -## Example: Using a backoff strategy +### Example: Using a backoff strategy When retrying failed RPCs, use a backoff strategy to avoid overwhelming the From 77fbd50c4071c213eefc864bd82f172c304d2290 Mon Sep 17 00:00:00 2001 From: ulas Date: Fri, 7 Apr 2023 01:30:31 +0300 Subject: [PATCH 3/9] fixes according the reviews, added godoc links to docstrings --- Documentation/anti-patterns.md | 43 +++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/Documentation/anti-patterns.md b/Documentation/anti-patterns.md index b2402316d404..77e5d8d7073e 100644 --- a/Documentation/anti-patterns.md +++ b/Documentation/anti-patterns.md @@ -1,21 +1,21 @@ ## Anti-Patterns ### Dialing in gRPC -`grpc.Dial` is a function in the gRPC library that creates a virtual connection +[`grpc.Dial`](https://pkg.go.dev/google.golang.org/grpc#Dial) is a function in the gRPC library that creates a virtual connection from the gRPC client to the gRPC server. It takes a target URI (which can represent the name of a logical backend service and could resolve to multiple -actual addresses) and a list of options, and returns a `ClientConn` object that +actual addresses) and a list of options, and returns a [`ClientConn`](https://pkg.go.dev/google.golang.org/grpc#ClientConn) object that represents the connection to the server. The `ClientConn` contains one or more actual connections to real server backends and attempts to keep these connections healthy by automatically reconnecting to them when they break. The `Dial` function can also be configured with various options to customize the behavior of the client connection. For example, developers could -use options such a `WithTransportCredentials` to configure the transport +use options such a [`WithTransportCredentials`](https://pkg.go.dev/google.golang.org/grpc#WithTransportCredentials) to configure the transport credentials to use. While `Dial` is commonly referred to as a "dialing" function, it doesn't -actually perform the low-level network dialing operation like `net.Dial` would. +actually perform the low-level network dialing operation like [`net.Dial`](https://pkg.go.dev/net#Dial) would. Instead, it creates a virtual connection from the gRPC client to the gRPC server. @@ -23,13 +23,13 @@ client to the gRPC server. the ClientConn object to manage and maintain that connection over time. This is why errors encountered during the initial connection are no different from those that occur later on, and why it's important to handle errors from RPCs rather -than relying on options like `FailOnNonTempDialError`, `WithBlock`, and -`WithReturnConnectionError`. +than relying on options like [`FailOnNonTempDialError`](https://pkg.go.dev/google.golang.org/grpc#FailOnNonTempDialError), [`WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), and +[`WithReturnConnectionError`](https://pkg.go.dev/google.golang.org/grpc#WithReturnConnectionError). In fact, `Dial` does not always establish a connection to servers by default. The connection behavior is determined by the load balancing policy being used. For instance, an "active" load balancing policy such as Round Robin attempts to maintain a constant connection, while the default "pick first" -policy delays connection until an RPC is executed. To explicitly initiate a connection, you can employ -the WithBlock option. +policy delays connection until an RPC is executed.Instead of using the WithBlock option, which may not be recommended +in some cases, you can call the [`ClientConn.Connect`](https://pkg.go.dev/google.golang.org/grpc#ClientConn.Connect) method to explicitly initiate a connection. ### Using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` @@ -40,6 +40,13 @@ failures at dial time. However, we strongly discourage developers from using these options, as they can introduce race conditions and result in unreliable and difficult-to-debug code. +One of the most important reasons for avoiding these options, which is often overlooked, +is that connections can fail at any point in time. This means that you need to handle +RPC failures caused by connection issues, regardless of whether a connection was never +established in the first place, or if it was created and then immediately lost. +Implementing proper error handling for RPCs is crucial for maintaining the reliability +and stability of your gRPC communication. + ### Why we discourage using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` When a client attempts to connect to a gRPC server, it can encounter a variety @@ -71,11 +78,18 @@ gRPC: - Always check for error responses from RPCs and handle them appropriately. - Use the `status` field of the error response to determine the type of error that occurred. -- When retrying failed RPCs, use a backoff strategy to avoid - overwhelming the server or exacerbating network issues. +- When retrying failed RPCs, consider using the built-in retry mechanism provided by gRPC-Go, + if available, instead of manually implementing retries. Refer to the [gRPC-Go retry example + documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/retry/README.md) + for more information. - Avoid using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError`, as these options can introduce race conditions and result in unreliable and difficult-to-debug code. +- If making the outgoing RPC in order to handle an incoming RPC, be sure to + translate the status code before returning the error from your method handler. + For example, if the error is an `INVALID_ARGUMENT` error, that probably means your + service has a bug (otherwise it shouldn't have triggered this error), in which + case `INTERNAL` is more appropriate to return back to your users. ### Example: Handling errors from an RPC @@ -136,11 +150,12 @@ server or exacerbating network issues: var res *MyResponse var err error +// If the user doesn't have a context with a deadline, create one +ctx, cancel := context.WithTimeout(context.Background(), time.Second) +defer cancel() + // Retry the RPC call a maximum number of times for i := 0; i < maxRetries; i++ { - // Create a context with a timeout of one second - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() // Make the RPC call res, err = client.MyRPC(ctx, &MyRequest{}) @@ -172,7 +187,7 @@ log.Printf("MyRPC response: %v", res) ## Conclusion -The `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` +The [`FailOnNonTempDialError`](https://pkg.go.dev/google.golang.org/grpc#FailOnNonTempDialError), [`WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), and [`WithReturnConnectionError`](https://pkg.go.dev/google.golang.org/grpc#WithReturnConnectionError) options are designed to handle errors at dial time, but they can introduce race conditions and result in unreliable and difficult-to-debug code. Instead of relying on these options, we strongly encourage developers to rely on errors From 45e481efa1af56964a989d285578634fd0ae2109 Mon Sep 17 00:00:00 2001 From: ulas Date: Fri, 7 Apr 2023 03:10:27 +0300 Subject: [PATCH 4/9] wrapped cols at 80 --- Documentation/anti-patterns.md | 87 +++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/Documentation/anti-patterns.md b/Documentation/anti-patterns.md index 77e5d8d7073e..4393b9f96fb7 100644 --- a/Documentation/anti-patterns.md +++ b/Documentation/anti-patterns.md @@ -1,35 +1,43 @@ ## Anti-Patterns ### Dialing in gRPC -[`grpc.Dial`](https://pkg.go.dev/google.golang.org/grpc#Dial) is a function in the gRPC library that creates a virtual connection -from the gRPC client to the gRPC server. It takes a target URI (which can -represent the name of a logical backend service and could resolve to multiple -actual addresses) and a list of options, and returns a [`ClientConn`](https://pkg.go.dev/google.golang.org/grpc#ClientConn) object that +[`grpc.Dial`](https://pkg.go.dev/google.golang.org/grpc#Dial) is a function in +the gRPC library that creates a virtual connection from the gRPC client to the +gRPC server. It takes a target URI (which can represent the name of a logical +backend service and could resolve to multiple actual addresses) and a list of +options, and returns a +[`ClientConn`](https://pkg.go.dev/google.golang.org/grpc#ClientConn) object that represents the connection to the server. The `ClientConn` contains one or more actual connections to real server backends and attempts to keep these connections healthy by automatically reconnecting to them when they break. -The `Dial` function can also be configured with various options to -customize the behavior of the client connection. For example, developers could -use options such a [`WithTransportCredentials`](https://pkg.go.dev/google.golang.org/grpc#WithTransportCredentials) to configure the transport -credentials to use. +The `Dial` function can also be configured with various options to customize the +behavior of the client connection. For example, developers could use options +such a +[`WithTransportCredentials`](https://pkg.go.dev/google.golang.org/grpc#WithTransportCredentials) +to configure the transport credentials to use. While `Dial` is commonly referred to as a "dialing" function, it doesn't -actually perform the low-level network dialing operation like [`net.Dial`](https://pkg.go.dev/net#Dial) would. -Instead, it creates a virtual connection from the gRPC -client to the gRPC server. +actually perform the low-level network dialing operation like +[`net.Dial`](https://pkg.go.dev/net#Dial) would. Instead, it creates a virtual +connection from the gRPC client to the gRPC server. -`Dial` does initiate the process of connecting to the server, but it uses -the ClientConn object to manage and maintain that connection over time. This is -why errors encountered during the initial connection are no different from those +`Dial` does initiate the process of connecting to the server, but it uses the +ClientConn object to manage and maintain that connection over time. This is why +errors encountered during the initial connection are no different from those that occur later on, and why it's important to handle errors from RPCs rather -than relying on options like [`FailOnNonTempDialError`](https://pkg.go.dev/google.golang.org/grpc#FailOnNonTempDialError), [`WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), and +than relying on options like +[`FailOnNonTempDialError`](https://pkg.go.dev/google.golang.org/grpc#FailOnNonTempDialError), +[`WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), and [`WithReturnConnectionError`](https://pkg.go.dev/google.golang.org/grpc#WithReturnConnectionError). -In fact, `Dial` does not always establish a connection to servers by default. The connection -behavior is determined by the load balancing policy being used. For instance, an "active" -load balancing policy such as Round Robin attempts to maintain a constant connection, while the default "pick first" -policy delays connection until an RPC is executed.Instead of using the WithBlock option, which may not be recommended -in some cases, you can call the [`ClientConn.Connect`](https://pkg.go.dev/google.golang.org/grpc#ClientConn.Connect) method to explicitly initiate a connection. +In fact, `Dial` does not always establish a connection to servers by default. +The connection behavior is determined by the load balancing policy being used. +For instance, an "active" load balancing policy such as Round Robin attempts to +maintain a constant connection, while the default "pick first" policy delays +connection until an RPC is executed. Instead of using the WithBlock option, which +may not be recommended in some cases, you can call the +[`ClientConn.Connect`](https://pkg.go.dev/google.golang.org/grpc#ClientConn.Connect) +method to explicitly initiate a connection. ### Using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` @@ -40,12 +48,13 @@ failures at dial time. However, we strongly discourage developers from using these options, as they can introduce race conditions and result in unreliable and difficult-to-debug code. -One of the most important reasons for avoiding these options, which is often overlooked, -is that connections can fail at any point in time. This means that you need to handle -RPC failures caused by connection issues, regardless of whether a connection was never -established in the first place, or if it was created and then immediately lost. -Implementing proper error handling for RPCs is crucial for maintaining the reliability -and stability of your gRPC communication. +One of the most important reasons for avoiding these options, which is often +overlooked, is that connections can fail at any point in time. This means that +you need to handle RPC failures caused by connection issues, regardless of +whether a connection was never established in the first place, or if it was +created and then immediately lost. Implementing proper error handling for RPCs +is crucial for maintaining the reliability and stability of your gRPC +communication. ### Why we discourage using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError` @@ -75,21 +84,24 @@ By handling errors from RPCs correctly, developers can write more reliable and robust gRPC applications. Here are some best practices for error handling in gRPC: -- Always check for error responses from RPCs and handle them appropriately. +- Always check for error responses from RPCs and handle them appropriately. - Use the `status` field of the error response to determine the type of error that occurred. -- When retrying failed RPCs, consider using the built-in retry mechanism provided by gRPC-Go, - if available, instead of manually implementing retries. Refer to the [gRPC-Go retry example +- When retrying failed RPCs, consider using the built-in retry mechanism +provided by gRPC-Go, + if available, instead of manually implementing retries. Refer to the [gRPC-Go + retry example documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/retry/README.md) for more information. -- Avoid using `FailOnNonTempDialError`, `WithBlock`, and `WithReturnConnectionError`, +- Avoid using `FailOnNonTempDialError`, `WithBlock`, and +`WithReturnConnectionError`, as these options can introduce race conditions and result in unreliable and difficult-to-debug code. - If making the outgoing RPC in order to handle an incoming RPC, be sure to translate the status code before returning the error from your method handler. - For example, if the error is an `INVALID_ARGUMENT` error, that probably means your - service has a bug (otherwise it shouldn't have triggered this error), in which - case `INTERNAL` is more appropriate to return back to your users. + For example, if the error is an `INVALID_ARGUMENT` error, that probably means + your service has a bug (otherwise it shouldn't have triggered this error), in + which case `INTERNAL` is more appropriate to return back to your users. ### Example: Handling errors from an RPC @@ -110,14 +122,13 @@ if err != nil { // Use the response as appropriate log.Printf("MyRPC response: %v", res) - ``` To determine the type of error that occurred, you can use the status field of the error response: -```go +```go resp, err := client.MakeRPC(context.Background(), request) if err != nil { status, ok := status.FromError(err) @@ -181,13 +192,15 @@ if err != nil { // Use the response as appropriate log.Printf("MyRPC response: %v", res) - ``` ## Conclusion -The [`FailOnNonTempDialError`](https://pkg.go.dev/google.golang.org/grpc#FailOnNonTempDialError), [`WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), and [`WithReturnConnectionError`](https://pkg.go.dev/google.golang.org/grpc#WithReturnConnectionError) +The +[`FailOnNonTempDialError`](https://pkg.go.dev/google.golang.org/grpc#FailOnNonTempDialError), +[`WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), and +[`WithReturnConnectionError`](https://pkg.go.dev/google.golang.org/grpc#WithReturnConnectionError) options are designed to handle errors at dial time, but they can introduce race conditions and result in unreliable and difficult-to-debug code. Instead of relying on these options, we strongly encourage developers to rely on errors From 68ab28ccc1e13cfdd3e28e0931eb132ba1b6aa62 Mon Sep 17 00:00:00 2001 From: ulas Date: Fri, 7 Apr 2023 03:25:58 +0300 Subject: [PATCH 5/9] added documentation reference --- dialoptions.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dialoptions.go b/dialoptions.go index e9d6852fd2c1..4116b7de019d 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -295,6 +295,8 @@ func withBackoff(bs internalbackoff.Strategy) DialOption { // WithBlock returns a DialOption which makes callers of Dial block until the // underlying connection is up. Without this, Dial returns immediately and // connecting the server happens in background. +// For more information about this anti-pattern +// see https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. func WithBlock() DialOption { return newFuncDialOption(func(o *dialOptions) { o.block = true @@ -310,6 +312,8 @@ func WithBlock() DialOption { // // Notice: This API is EXPERIMENTAL and may be changed or removed in a // later release. +// For more information about this anti-pattern +// see https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. func WithReturnConnectionError() DialOption { return newFuncDialOption(func(o *dialOptions) { o.block = true @@ -452,6 +456,8 @@ func withBinaryLogger(bl binarylog.Logger) DialOption { // // Notice: This API is EXPERIMENTAL and may be changed or removed in a // later release. +// For more information about this anti-pattern +// see https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. func FailOnNonTempDialError(f bool) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.FailOnNonTempDialError = f From 16ce5d4cc115e8016c1a0e91d8937912cded67d6 Mon Sep 17 00:00:00 2001 From: ulas Date: Fri, 7 Apr 2023 03:39:11 +0300 Subject: [PATCH 6/9] fixes --- Documentation/anti-patterns.md | 2 +- dialoptions.go | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Documentation/anti-patterns.md b/Documentation/anti-patterns.md index 4393b9f96fb7..cafb9ee7a666 100644 --- a/Documentation/anti-patterns.md +++ b/Documentation/anti-patterns.md @@ -88,7 +88,7 @@ gRPC: - Use the `status` field of the error response to determine the type of error that occurred. - When retrying failed RPCs, consider using the built-in retry mechanism -provided by gRPC-Go, + provided by gRPC-Go, if available, instead of manually implementing retries. Refer to the [gRPC-Go retry example documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/retry/README.md) diff --git a/dialoptions.go b/dialoptions.go index 4116b7de019d..5235e1a66d5f 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -295,8 +295,7 @@ func withBackoff(bs internalbackoff.Strategy) DialOption { // WithBlock returns a DialOption which makes callers of Dial block until the // underlying connection is up. Without this, Dial returns immediately and // connecting the server happens in background. -// For more information about this anti-pattern -// see https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. +// Use of this feature is not recommended. For more information, please see: https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. func WithBlock() DialOption { return newFuncDialOption(func(o *dialOptions) { o.block = true @@ -308,12 +307,11 @@ func WithBlock() DialOption { // the context.DeadlineExceeded error. // Implies WithBlock() // +// Use of this feature is not recommended. For more information, please see: https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. // # Experimental // // Notice: This API is EXPERIMENTAL and may be changed or removed in a // later release. -// For more information about this anti-pattern -// see https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. func WithReturnConnectionError() DialOption { return newFuncDialOption(func(o *dialOptions) { o.block = true @@ -452,12 +450,11 @@ func withBinaryLogger(bl binarylog.Logger) DialOption { // FailOnNonTempDialError only affects the initial dial, and does not do // anything useful unless you are also using WithBlock(). // +// Use of this feature is not recommended. For more information, please see: https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. // # Experimental // // Notice: This API is EXPERIMENTAL and may be changed or removed in a // later release. -// For more information about this anti-pattern -// see https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. func FailOnNonTempDialError(f bool) DialOption { return newFuncDialOption(func(o *dialOptions) { o.copts.FailOnNonTempDialError = f From d7e7d647b7d2aef49d1bb1faae142d8efd6883d6 Mon Sep 17 00:00:00 2001 From: ulas Date: Fri, 7 Apr 2023 03:41:52 +0300 Subject: [PATCH 7/9] fix on lining --- Documentation/anti-patterns.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/anti-patterns.md b/Documentation/anti-patterns.md index cafb9ee7a666..fef7c94841b3 100644 --- a/Documentation/anti-patterns.md +++ b/Documentation/anti-patterns.md @@ -94,7 +94,7 @@ gRPC: documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/retry/README.md) for more information. - Avoid using `FailOnNonTempDialError`, `WithBlock`, and -`WithReturnConnectionError`, + `WithReturnConnectionError`, as these options can introduce race conditions and result in unreliable and difficult-to-debug code. - If making the outgoing RPC in order to handle an incoming RPC, be sure to From 2585a3496184d3dfe12a3f5ea1eb7481f2d43850 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Thu, 6 Apr 2023 17:53:49 -0700 Subject: [PATCH 8/9] Clean up formatting --- dialoptions.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dialoptions.go b/dialoptions.go index 5235e1a66d5f..cdc8263bda65 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -295,7 +295,9 @@ func withBackoff(bs internalbackoff.Strategy) DialOption { // WithBlock returns a DialOption which makes callers of Dial block until the // underlying connection is up. Without this, Dial returns immediately and // connecting the server happens in background. -// Use of this feature is not recommended. For more information, please see: https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. +// +// Use of this feature is not recommended. For more information, please see: +// https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md func WithBlock() DialOption { return newFuncDialOption(func(o *dialOptions) { o.block = true @@ -307,7 +309,9 @@ func WithBlock() DialOption { // the context.DeadlineExceeded error. // Implies WithBlock() // -// Use of this feature is not recommended. For more information, please see: https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. +// Use of this feature is not recommended. For more information, please see: +// https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md +// // # Experimental // // Notice: This API is EXPERIMENTAL and may be changed or removed in a @@ -450,7 +454,9 @@ func withBinaryLogger(bl binarylog.Logger) DialOption { // FailOnNonTempDialError only affects the initial dial, and does not do // anything useful unless you are also using WithBlock(). // -// Use of this feature is not recommended. For more information, please see: https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md. +// Use of this feature is not recommended. For more information, please see: +// https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md +// // # Experimental // // Notice: This API is EXPERIMENTAL and may be changed or removed in a From 914b161b5b22a85fe5f19836ff9db1954a973d1a Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Thu, 6 Apr 2023 17:54:40 -0700 Subject: [PATCH 9/9] Clean up line wraps --- Documentation/anti-patterns.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Documentation/anti-patterns.md b/Documentation/anti-patterns.md index fef7c94841b3..08469fc179f7 100644 --- a/Documentation/anti-patterns.md +++ b/Documentation/anti-patterns.md @@ -88,15 +88,13 @@ gRPC: - Use the `status` field of the error response to determine the type of error that occurred. - When retrying failed RPCs, consider using the built-in retry mechanism - provided by gRPC-Go, - if available, instead of manually implementing retries. Refer to the [gRPC-Go - retry example + provided by gRPC-Go, if available, instead of manually implementing retries. + Refer to the [gRPC-Go retry example documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/retry/README.md) for more information. - Avoid using `FailOnNonTempDialError`, `WithBlock`, and - `WithReturnConnectionError`, - as these options can introduce race conditions and result in unreliable and - difficult-to-debug code. + `WithReturnConnectionError`, as these options can introduce race conditions and + result in unreliable and difficult-to-debug code. - If making the outgoing RPC in order to handle an incoming RPC, be sure to translate the status code before returning the error from your method handler. For example, if the error is an `INVALID_ARGUMENT` error, that probably means