Skip to content

Conversation

@fantix
Copy link
Member

@fantix fantix commented Nov 10, 2022

This improves the error output when an ErrorResponse is received upon Sync after the client connection is idle over _ping_wait_time. The error was:

Traceback (most recent call last):
  File "edgedb/blocking_client.py", line 155, in raw_query
    await self._protocol._sync()
  File "edgedb/protocol/protocol.pyx", line 712, in _sync
  File "edgedb/protocol/protocol.pyx", line 1071, in edgedb.protocol.protocol.SansIOProtocol.fallthrough
edgedb.errors.ProtocolError: unexpected message type 'E'

But this cannot explain or fix the server issue why an ErrorResponse is returned upon Sync.

The server will send an IdleSessionTimeoutError when killing idle connections:

https://github.com/edgedb/edgedb/blob/2b426dccf6f2c4af60915a8ee47a41eff1f6a00c/edb/server/protocol/binary.pyx#L371-L378

Depending on the idle time (comparing to socket write timeout), the blocking client may still receive this error on ping. In this case, we should just reconnect and recover.

Refs #365

@fantix fantix requested a review from 1st1 November 10, 2022 14:58
@fantix fantix marked this pull request as ready for review November 14, 2022 15:29
@fantix fantix merged commit 86b5183 into master Nov 18, 2022
@fantix fantix deleted the ping branch November 18, 2022 14:05
fantix added a commit that referenced this pull request Nov 23, 2022
* Handle IdleSessionTimeoutError in blocking client reconnect
fantix added a commit that referenced this pull request Nov 23, 2022
Changes
=======

* Output pretty error if possible (#399)
  (by @fantix in a2bec18 for #399)

* Codegen: allow providing a path after --file (#400)
  (by @fantix in 6bce57e for #400)

Fixes
=====

* Handle ErrorResponse in ping (#393)
  (by @fantix in 8b28947 for #393)

* Disallow None in elements of array argument
  (by @fantix in 26fb6d8)

Docs
====

* Remove references to unix-domain sockets
  (by @quinchs in 4b8bec6)
@fantix fantix mentioned this pull request Nov 23, 2022
@aljazerzen aljazerzen mentioned this pull request Feb 23, 2024
This was referenced Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants