Skip to content

Conversation

@IgorSilvestre
Copy link
Contributor

@IgorSilvestre IgorSilvestre commented Dec 15, 2025

Why

Feature asked by this issue:

What

  • Introduce DialOptions.MaxErrorResponseBodyBytes so callers can opt into more (or zero) error-body
    bytes when the handshake fails, enabling better branching on status/body while keeping the default
    1024-byte limit.
  • Preserve existing success-path behavior and safety: response bodies stay nil on success; 3s timeout
    still guards error-body reads; disabled option closes without reading.

Key Changes

  • dial.go: use new option to bound error-body capture; skip capture if set < 0; guard nil bodies;
    still close original body with 3s fail-safe.
  • dial_test.go: new/updated tests covering default cap, custom cap, disabled capture (ensures close),
    status code presence, and nil-body safety.

Testing

I did some testing with a dummy server responding a fixed status 400 and a 1mb body:
image

image

@IgorSilvestre IgorSilvestre changed the title Igorsilvestre/issue #528 - introduces an option for a custom body size in websocket.Dial function ISSUE #528 - introduces an option for a custom body size in websocket.Dial function Dec 15, 2025
@mafredri
Copy link
Member

Closing this as it does not address #528, which does not request an increase in body size.

@mafredri mafredri closed this Dec 16, 2025
@mafredri mafredri changed the title ISSUE #528 - introduces an option for a custom body size in websocket.Dial function feat: introduces an option for a custom body size in websocket.Dial function Dec 16, 2025
@IgorSilvestre
Copy link
Contributor Author

IgorSilvestre commented Dec 16, 2025

Closing this as it does not address #528, which does not request an increase in body size.

Fair, I assumed in

and ideally the body too. Is there any chance of modifying Dial to return this information"

that he would want to return more of the body, seeing its already returning 1024b.

@mafredri
Copy link
Member

@IgorSilvestre no worries. Thanks for attempting a fix. We are very conservative with expanding API surface in this project and there would have to be a real and obvious need if we were to add something.

@IgorSilvestre
Copy link
Contributor Author

IgorSilvestre commented Dec 16, 2025

@IgorSilvestre no worries. Thanks for attempting a fix. We are very conservative with expanding API surface in this project and there would have to be a real and obvious need if we were to add something.

On reading other issues I saw that minimizing the API is something important for you, no problem.

I really like this project, if ever need a hand with something just ask.

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.

2 participants