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

Reject create(Uni|Bi)directionalStream() on stream ID exhaustion. #528

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

jan-ivar
Copy link
Member

@jan-ivar jan-ivar commented Jul 19, 2023

Fixes #446.


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Jul 19, 2023, 7:52 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator at 
 [no address given] to inform them of the time this error occurred,
 and the actions you performed just before this error.</p>
<p>More information about this error may be available
in the server error log.</p>
<hr>
<address>Apache/2.4.10 (Debian) Server at api.csswg.org Port 443</address>
</body></html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@vasilvv
Copy link
Contributor

vasilvv commented Jul 19, 2023

I'm a little bit confused. I remember last time we discussed this in #469, the conclusion was that we're supposed to block the promise until the next stream is available. Have I missed something?

@jan-ivar
Copy link
Member Author

My understanding from the May 23 meeting notes and #446 (comment) was that we wanted to rethink this:

TODO:

  • Need a PR to reject createBi/UniDirectionStream() in a reliable way, when MAX stream limit is hit. Let's app drop its request, OR
  • Fire some limitreached event

I believe the thinking was that blocking the promise made it difficult for the application to detect that the MAX_STREAM_LIMIT has been hit, and that waiting wasn't always the right cause of action. Rejecting instead would let the application know what happened so it can decide what to do next.

@ricea
Copy link
Contributor

ricea commented Jul 21, 2023

But how can the app know when the limit has been increased and it is safe to call Create*Stream() again?

@jan-ivar jan-ivar merged commit 40e83a0 into w3c:main Sep 26, 2023
1 check passed
@jan-ivar jan-ivar deleted the streamlimit branch September 26, 2023 14:13
github-actions bot added a commit that referenced this pull request Sep 26, 2023
SHA: 40e83a0
Reason: push, by jan-ivar

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

MAX_STREAM limit (Client→Server) (still need an event when stream ids become available again)
3 participants