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

Bug report — Rewriting a WebSocket response throws exception due to unsupported status code 101. #3047

Open
sassanh opened this issue Nov 2, 2024 · 9 comments
Labels
feature request Request for Workers team to add a feature

Comments

@sassanh
Copy link

sassanh commented Nov 2, 2024

If I try to instantiate a response based on another response, with status code 101, returned by a DurableObject, I get this error:
Responses may only be constructed with status codes in the range 200 to 599, inclusive.

This is causing an issue in next-on-pages as reported here

I don't know if it should be addressed here, or next-on-pages should put an exception for this case to directly return the response coming from the DurableObject and return it without re-instantiation.

@kentonv
Copy link
Member

kentonv commented Nov 3, 2024

As a work-around, you can change the status to 200 -- because it has a webSocket property, the system will change it back to 101.

@sassanh
Copy link
Author

sassanh commented Nov 3, 2024

@kentonv Thanks for the quick response! I will try that, for now I'm using this minor patch on next-on-pages

Do you know if it should be taken care in workerd by allowing status code 101, or by providing an api for handling special cases when response is being forwarded from other services like durable objects, or it should be taken care in next-on-pages and it is not going to be supported to instantiate a response with status code 101 in workerd?

@jasnell
Copy link
Member

jasnell commented Nov 3, 2024

Supporting 1xx interim responses in workers is something we've discussed but is not trivial as it is something that would need to be implemented deep throughout our stack. I do want to make this happen but we'll need to discuss internally around priority and feasibility. I'm going to change this issue to a feature request as it's not really a bug as much as a limitation of the current design/implementation.

@jasnell jasnell added the feature request Request for Workers team to add a feature label Nov 3, 2024
@jasnell jasnell changed the title 🐛 Bug Report — Runtime APIs - Responses may only be constructed with status codes in the range 200 to 599, inclusive. Feature Request — Runtime APIs - Responses may only be constructed with status codes in the range 200 to 599, inclusive. Nov 3, 2024
@sassanh
Copy link
Author

sassanh commented Nov 4, 2024

@jasnell Thanks! That makes sense.

So to wrap it up and avoid misinterpretation of this issue in the future, if I understand it correctly:

  1. We are not looking for WebSocket support in workerd here, we just want to be able to pass WebSocket responses coming from durable objects, for example.
  2. Currently, workerd can pass WebSocket responses without any workarounds or hacks, with its official API, as long as one doesn't try to modify it in workerd.
  3. If one wants to add headers to the WebSocket Response object coming from a durable object, for example, or change its status code to something in the range 1xx, that's not supported in the official API at the moment, but as a workaround, in the case the desired status code is 101 and not any arbitrary number in the range 1xx, they can set the status code to 200, and the system will change it to 101 automatically.

@kentonv kentonv changed the title Feature Request — Runtime APIs - Responses may only be constructed with status codes in the range 200 to 599, inclusive. Bug report — Rewriting a WebSocket response throws exception due to unsupported status code 101. Nov 11, 2024
@kentonv
Copy link
Member

kentonv commented Nov 11, 2024

I've changed the title of this bug to better reflect the specific bug here.

@jasnell the ask is NOT to support 1xx in general. The problem here is that fetch() itself returns a Response with status 101 when the response is a WebSocket response, and attempts to rewrite that response (e.g. change a header) fail because we throw an exception about not supporting 1xx status codes.

This is a bug which we should fix. We should simply say that if the Response is specifically a WebSocket response (its webSocket property is non-null), then we allow it to have status 101.

@jasnell
Copy link
Member

jasnell commented Nov 11, 2024

This is a bug which we should fix. We should simply say that if the Response is specifically a WebSocket response (its webSocket property is non-null), then we allow it to have status 101.

SGTM

@sassanh
Copy link
Author

sassanh commented Nov 11, 2024

Thanks!

This might help others with similar issues:

My original challenge was serving the WebSocket endpoint on the same subdomain as my web application, which led me to try proxying responses. Even with a workaround to deal with the issue reported here, I ran into the issue of the connection closing after 100 seconds of inactivity due to hibernation (noted in the docs but a little easy to miss).

Then, I discovered the routes feature, a handy tool that allows serving different workerd instances on different paths without needing to proxy WebSocket responses.

@wighawag
Copy link

I am hitting this issue on deployed cf worker

I have a code liek this in my Durable Object

const webSocketPair = new WebSocketPair();
const [client, server] = Object.values(webSocketPair);
this.state.acceptWebSocket(server);
return new Response(null, {
	status: 101,
	webSocket: client,
});

and the worker forward that back to the client

With that I get

RangeError: Responses may only be constructed with status codes in the range 200 to 599, inclusive.

If I set status to be 200 I get

RangeError: Responses with a WebSocket must have status code 101.

@kentonv
Copy link
Member

kentonv commented Dec 18, 2024

Hmm now I'm confused, the code actually looks correct.

if (webSocket == kj::none) {
JSG_REQUIRE(statusCode >= 200 && statusCode <= 599, RangeError,
"Responses may only be constructed with status codes in the range 200 to 599, inclusive.");
} else {
JSG_REQUIRE(
statusCode == 101, RangeError, "Responses with a WebSocket must have status code 101.");
}

So my work-around suggestion earlier is actually wrong. But AFAICT it shouldn't be needed, the runtime code does the right thing, and will allow (in fact, require) a status code of 101 whenever a WebSocket is present.

Can someone please provide a complete, self-contained reproduction of the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for Workers team to add a feature
Projects
None yet
Development

No branches or pull requests

4 participants