Skip to content

Conversation

DanielleMaywood
Copy link
Contributor

@DanielleMaywood DanielleMaywood commented Aug 15, 2025

This change allows detecting when a message exceeds the maximum allowed size
during reads via errors.Is(err, ErrMessageTooBig). Previously, this condition
required string matching against the error message. For backwards compatibility,
the old message is preserved in the error chain.

@DanielleMaywood DanielleMaywood marked this pull request as ready for review August 15, 2025 12:07
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Deferring approval to @mafredri

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Considering this code-path closes the connection, what use-case are we addressing by detecting the specific error?

@DanielleMaywood
Copy link
Contributor Author

Considering this code-path closes the connection, what use-case are we addressing by detecting the specific error?

The exact use-case is for better logging here:
https://github.com/coder/coder/blob/205eb29e60cc3b3519b4ff0247d191b0c87e4d75/codersdk/wsjson/decoder.go#L39-L41

It was previously decided to log at a debug log level. Assuming we want to keep that original behavior, but raise it for this specific error, we want to be able to match against it properly (without using string.Contains on the error).

@mafredri mafredri self-assigned this Sep 1, 2025
@mafredri mafredri changed the title refactor: replace fmt.Errorf with MessageTooBigError refactor: add ErrMessageTooBig sentinel error for limited reads Sep 4, 2025
@DanielleMaywood DanielleMaywood merged commit 7d7c644 into master Sep 5, 2025
4 checks passed
@mafredri mafredri deleted the danielle/type-error-return branch September 5, 2025 08:21
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