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 getStats() promise when the connection is closed/failed. #558

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

vasilvv
Copy link
Contributor

@vasilvv vasilvv commented Oct 20, 2023

Fixes #557


Preview | Diff

index.bs Outdated
Comment on lines 990 to 991
1. If |transport|.{{[[State]]}} is `"closed"` or `"failed"`,
[=reject=] |p| with an {{InvalidStateError}} and abort these steps.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should do the same state check in the synchronous steps (since the timing of promise state is synchronously observable in a JS debugger).

@jan-ivar
Copy link
Member

jan-ivar commented Oct 24, 2023

Meeting:

  • Reject on failed
  • Record final stats on close and return those after close.
  • Block promise until connected

@jan-ivar jan-ivar merged commit 2c82082 into w3c:main Nov 21, 2023
1 check passed
github-actions bot added a commit that referenced this pull request Nov 21, 2023
SHA: 2c82082
Reason: push, by jan-ivar

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 22, 2024
w3c/webtransport#558 requires providing stats
after connection has been closed; this ensures we always have some valid
stats as soon as the connection is open, and also that we have the most
recent stats at the moment the connection is closed.

Change-Id: Ic94ea86a6b2bb5f84a26fa8c1f3196549bb20368
Bug: 1519563
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5197933
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Victor Vasiliev <vasilvv@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1250521}
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.

getStats() being rejected
2 participants