Skip to content

Support response header modifications with streaming#572

Open
mdmower-csnw wants to merge 1 commit into
villadora:masterfrom
CSNW:stream-res-headers
Open

Support response header modifications with streaming#572
mdmower-csnw wants to merge 1 commit into
villadora:masterfrom
CSNW:stream-res-headers

Conversation

@mdmower-csnw

Copy link
Copy Markdown
  • Add option stream so that users can opt into response body streaming while also modifying response headers.
    • Gating this with an option ensures the application behavior does not change for existing users of the application. It also means there is a better chance users will see and consider the warning "be careful about modifying response headers that describe the body".
    • This capability is safe because response headers are fully read by the time userResHeaderDecorator runs.
  • Add tests that ensure response header modifications still work when streaming is enabled.
  • Update readme.

- Add option `stream` so that users can opt into response body streaming
  while also modifying response headers.
  - Gating this with an option ensures the application behavior does not
    change for existing users of the application. It also means there is
    a better chance users will see and consider the warning "be careful
    about modifying response headers that describe the body".
  - This capability is safe because response headers are fully read by
    the time `userResHeaderDecorator` runs.
- Add tests that ensure response header modifications still work when
  streaming is enabled.
- Update readme.
@mdmower-csnw

Copy link
Copy Markdown
Author

@monkpow - Is this a feature you'd be willing to consider for the proxy?

Comment thread lib/resolveOptions.js
Comment on lines +36 to +41
if (stream && resolved.skipToNextHandlerFilter) {
throw new Error(
'skipToNextHandlerFilter inspects a buffered response and cannot be ' +
'combined with stream:true.'
);
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

An argument could be made for allowing simultaneous use of stream and skipToNextHandlerFilter, but the user would have to recognize that reading the body in skipToNextHandlerFilter is a no-no.

@monkpow

monkpow commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

@mdmower-csnw Thanks so much for the PR. I'm on summer break at the moment, so it will take me at least a week to review in detail, but I like what I see so far.

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