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

Improve examples. #182

Merged
merged 12 commits into from Feb 3, 2021
Merged

Improve examples. #182

merged 12 commits into from Feb 3, 2021

Conversation

jan-ivar
Copy link
Member

@jan-ivar jan-ivar commented Jan 22, 2021

@jan-ivar jan-ivar self-assigned this Jan 22, 2021
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@jan-ivar
Copy link
Member Author

jan-ivar commented Jan 22, 2021

Hmm, I added some error propagation for the in parallel steps, but I'm not happy with it, since it defers errors until the end.

@jan-ivar
Copy link
Member Author

Are errors likely to occur per stream, or are they all fatal to the transport?

@jan-ivar
Copy link
Member Author

Ended up reporting per stream, since the iteration on incomingUnidirectionalStreams itself should fail if the connection fails.

@jan-ivar
Copy link
Member Author

jan-ivar commented Jan 22, 2021

For those interested, I wrote a quick polyfill (demo) to verify this for await syntax:

for await (const stream of transport.incomingUnidirectionalStreams) {
  await stream.readable.pipeThrough(new TextDecoderStream("utf-8")).pipeTo(new WritableConsole());
}

I dunno if it's production ready or anything, but it appears to work right in Canary.

@jan-ivar
Copy link
Member Author

for await (const stream of transport.incomingUnidirectionalStreams) {
 await stream.readable.pipeThrough(new TextDecoderStream("utf-8")).pipeTo(new WritableConsole());
}

For those curious, the reason for not reading in parallel here is to avoid interleaving the text output, since it's going to a combined log.

Now that I think about it, that might be good to show in the text decoder example.

index.bs Outdated Show resolved Hide resolved
break;
}
}
} catch (e) {
addToEventLog('Error while sending data: ' + e, 'error');
addToEventLog(`Error while sending data: ${e}`, 'error');
Copy link
Member

Choose a reason for hiding this comment

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

We can't use <![CDATA[ blocks so that this quoting is not necessary?

@jan-ivar jan-ivar merged commit 8ed586d into w3c:master Feb 3, 2021
github-actions bot added a commit that referenced this pull request Feb 3, 2021
Improve examples.

SHA: 8ed586d
Reason: push, by @jan-ivar

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@aboba aboba mentioned this pull request Feb 15, 2021
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.

None yet

5 participants