connectrpc / connect-es

The TypeScript implementation of Connect: Protobuf RPC that works.
https://connectrpc.com/
Apache License 2.0
1.37k stars 79 forks source link

ConnectError: [unknown] network error #836

Closed akosyakov closed 1 year ago

akosyakov commented 1 year ago

Describe the bug

We are seeing in our auto error reporting from the browser application:

ConnectError: [unknown] network error
    at Function.value (/workspace/gitpod/node_modules/@bufbuild/connect/dist/esm/connect-error.js:72:19)
    at from (/workspace/gitpod/node_modules/@bufbuild/connect/dist/esm/protocol/run-call.js:103:35)

It seems some network issues are not recognized like Unavailable.

To Reproduce

Not sure. Something related to network issues. GCP load balancer resets connections each 10 mins, maybe related. It was also coming from server side streaming API, i.e. long running connection.

Environment (please complete the following information):

We are using 0.13.0 for all deps right now.

Additional context Add any other context about the problem here.

akosyakov commented 1 year ago

Yeah it happens when server-stream after 10mins when GCP resets the connection:

Screenshot 2023-09-20 at 17 12 30
akosyakov commented 1 year ago

It happens here:

Screenshot 2023-09-20 at 20 51 45
akosyakov commented 1 year ago

Is there a way to set timeout on all requests that they get closed a bit earlier with more expected error? Asking because I see different browser behaves differently in this case: https://github.com/kyma-project/busola/blob/ebad26a4f8fd86905dcaa43ada088f7cd15c8f80/src/shared/hooks/BackendAPI/useGet.js#L161-L165

Edit: it does not seem to be possible right now: https://github.com/connectrpc/connect-es/issues/799#issue-1875854361

srikrsna-buf commented 1 year ago

Hey! Your analysis seems accurate. Unfortunately, fetch doesn't provide distinguishable errors across browser implementations. Hence we can't reliably address this within connect-web.

The solution you arrived at seems reasonable, timeouts can be be set on a per rpc basis and retried. Once #799 is addressed they can be set at a transport level.

Closing this for now, feel free to reopen if needed.

akosyakov commented 11 months ago

@srikrsna-buf I'm considering to add instead server side timeout that all API clients have the same experience. Do you think it is sensible? feasible? common?

srikrsna-buf commented 11 months ago

Server side timeouts sound good to me as a way to set resource limits. If there are non web clients, no need to enforce the limit for them which would otherwise be acceptable.