Skip to content

Conversation

jonasfj
Copy link
Member

@jonasfj jonasfj commented Mar 26, 2025

I think this will fix issues where the connections are not closed correctly.

@jonasfj jonasfj requested review from isoos and sigurdm March 26, 2025 12:38
Comment on lines 244 to 246
// NOTE: Sadly, this can throw, it can even throw synchroniously,
// implying [Socket] from 'dart:io' fails to implement
// [StreamSink] as specified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// NOTE: Sadly, this can throw, it can even throw synchroniously,
// implying [Socket] from 'dart:io' fails to implement
// [StreamSink] as specified.
// NOTE: Sadly, this can throw, it can even throw synchroniously,
// implying [Socket] from 'dart:io' fails to implement
// [StreamSink] as specified.

If this is true we should add a issue number here.

Copy link
Collaborator

@sigurdm sigurdm left a comment

Choose a reason for hiding this comment

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

If we feel we need this band-aid I don't mind approving it - but it gives me the heebie-jeebies.

I don't like catch-alls if they can be avoided. They can cover up other issues

I despise having to catch StateErrors. It implies we are holding it wrong.

I think we have the responsibility to file tracking issues for sdk bugs.

Maybe we need a wrapper of the StreamSink that ignores adds after a close(), if we want a kill-switch for our stream. Or we keep track of all on-going addStream futures, and await them before closing.

@sigurdm sigurdm merged commit 50e7f54 into google:master Oct 17, 2025
3 checks passed
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.

3 participants