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

Report a distinct error code for sync connection timeouts #6932

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Aug 28, 2023

The same error code was used for both timeouts and other connection failures, and the error reported always had an empty message.

@tgoyne tgoyne requested a review from michael-wb August 28, 2023 22:30
@tgoyne tgoyne self-assigned this Aug 28, 2023
@cla-bot cla-bot bot added the cla: yes label Aug 28, 2023
Copy link
Contributor

@michael-wb michael-wb left a comment

Choose a reason for hiding this comment

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

LGTM - the two failures are known issues (audit: due to a new feature in baas server, compensating writes: occasionally fails due to missing MARK response)

Two tiny comments - we haven't really established a policy about adding new values to error_codes.h since we're not sure if anyone is relying on the numeric values (hopefully not), but this change should be fine.

CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines +76 to +85
RLM_ERR_SYNC_CONNECT_TIMEOUT = 1035,
RLM_ERR_SYNC_INVALID_SCHEMA_CHANGE = 1036,
RLM_ERR_SYNC_PERMISSION_DENIED = 1037,
RLM_ERR_SYNC_PROTOCOL_INVARIANT_FAILED = 1038,
RLM_ERR_SYNC_PROTOCOL_NEGOTIATION_FAILED = 1039,
RLM_ERR_SYNC_SERVER_PERMISSIONS_CHANGED = 1040,
RLM_ERR_SYNC_USER_MISMATCH = 1041,
RLM_ERR_TLS_HANDSHAKE_FAILED = 1042,
RLM_ERR_WRONG_SYNC_TYPE = 1043,
RLM_ERR_SYNC_WRITE_NOT_ALLOWED = 1044,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully no one is relying on the actual numeric values...

The same error code was used for both timeouts and other connection failures,
and the error reported always had an empty message.
@tgoyne tgoyne merged commit f47e0bf into master Aug 29, 2023
2 checks passed
@tgoyne tgoyne deleted the tg/sync-timeout-error branch August 29, 2023 18:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants