Comments (11)
CC @lrhn
from async.
For the moment, I'm going to just wrap the call in a try/catch/finally. Again, this is happening when the process is closing down.
from async.
The stream needs to be listened to once in order to forward the return value from StreamSubscription.cancel()
.
Here is one test that fails if we don't listen first:
async/test/stream_queue_test.dart
Lines 484 to 490 in 30e9ccc
I think in practice very few onCancel
callbacks return a useful value, and the documentation for StreamSubscription.cancel
says the future should only ever complete with null
.
Changing this behavior would be breaking. Given the current documentation it might be preferable to change it, but I'm not sure if it's worth the breaking change.
I think, though, in general we don't really want to support the use case of creating multiple StreamQueue
instances off the same non-broadcast stream as long as you only ever request events from one of them... We should probably take a look at whether we can do something different in the test package.
from async.
Yeah, my approach described above works for now, but it's definitely a hack.
An alternative that may make sense is to change the implementation of stdin
in dart:io
package to be a BroadcastStream(which, for what it's worth should be the default IMO). I can't think of bad behavior that could be caused by multiple consumption of STDIN. If someone can think of some, please let me know.
I talked to @mit-mit about having someone from Dart lang team look at this.
from async.
So the problem is that you have two different clients trying to use stdin
, you are just lucky that only one of them actually do something with it.
I guess the test code could be changed to not cancel the stream unless it has actually been used (it would then have to record that somewhere).
Personally, I'd probably initialize the StreamQueue
lazily the first time it's actually needed, instead of creating it ahead of time. Then you would only need to cancel the queue if it exists.
from async.
Yeah, lazy initialization is not very pretty in dart, but I've got a patch to do exactly this.
But I'm still confused by why you think we would be 'lucky' if someone used the STDIN stream only once. To be honest, I'm actually confused by the semantics of having a stream be only listened to once period. To me, that's like saying you can only only iterate a list once. The current API requires I know way too much about the implementation of every other thing in not only my program, but also all of it's dependencies - possibly even the Dart standard library.
Developers should be able to use a Stream and not worry about StateErrors that that stem from behavior outside of their own application code, especially since these sort of Effects are not transparent in the type system.
The following should probably be cross posted in an issue on dart-lang/sdk:
Some of this would be easier if you could ask a Stream whether it's been listened to or not, but with the current API, I need to wrap a call to listen()
for any stream I do not explicitly control with try
/catch
.
A more sensible API would be to have listen()
return a subscription if the listener is added, and null if not. That's a very simple, straightforward way for a developer to at least handle the case - which will be more elegant once we have NNTBD, and therefore the nullability will be encoded in the type signature. What's more, single subscription streams should probably be the exception rather than the default. Alternatively, Stream should be the name of the abstract class and the current Stream class should be called SinglecastStream or something similar.
I don't really want to create multiple StreamQueue
's off of a Singlecast stream. I just want the semantics of the cancel method to be maintained in the close method of StreamQueue
, direct from the docs for StreamSubscription.cancel
:
A returned future completes with a null value. If the cleanup throws, which it really shouldn't, the returned future completes with that error.
So, in my mind that means return the future from cancelling the current subscription if one exists, or a Future otherwise. So something roughly like this:
Future _cancel() {
if (_isDone) return null;
var future = _subscription != null ? Future.value(null) : _subscription.cancel();
_close();
return future;
}
from async.
just want the semantics of the cancel method to be maintained in the close method of
StreamQueue
, direct from the docs forStreamSubscription.cancel
I think that would be the right approach too - but I also don't think it's worth the breaking change.
from async.
@natebosch has there been development or alternative ideas on this issue?
from async.
@ened - I'll tag with "next breaking release" so that if we have some other compelling reason to bump the version here we can fix this along with it - as I mentioned earlier I don't think it's worth a major version release for just this change.
The recommendation is still to avoid wrapping a single subscriber stream in a StreamQueue
multiple times, and closing multiple times, when you must only listen once.
from async.
For the record, the behavior is intentional. We want the stream to be cleaned up when you close the stream queue.
In other situations, we've had requests to do exactly that because they would otherwise leak resources when a stream was not listened to and closed, so generally we let stream consumers always consume the stream, even if you read no events at all.
from async.
No change planned.
from async.
Related Issues (20)
- Change the default of `propagateCancel` argument in CancelableOperation.then HOT 2
- Reset method for AsyncMemoizer HOT 1
- Make it easier to safely hold a reference that can cancel an operation without holding the whole operation HOT 1
- Clarify `StreamQueue.next` will fail just after `hasNext` in API document.
- Consider to make second invocation of `streamQueue.hasNext` be postponed concluding the result until the first invocation of `q.next` , unless the stream is closed. HOT 6
- Deprecate StreamQueue.hasNext and StreamQueue.next
- Future.wait() but with Records HOT 4
- Bad State error while trying to reject a StreamQueueTransaction
- Dart 3 incompatibilty: `DelegatingStream<T> extends StreamView` but `StreamView` is `base class` HOT 5
- Add `whereNotNull` for `Stream`
- CancelableOperation value is not propagating errors, so they cannot be catched and app is crashing HOT 3
- There should be cancellable versions of Stream.firstWhere etc.
- Migrate the Result type to sealed classes HOT 2
- Make `ParallelWaitError` Include Error Details HOT 1
- Async Cache is caching exceptions HOT 5
- AsyncMemoizer is caching exceptions HOT 2
- Add an API wrapping runZonedGuarded that surfaces the first error HOT 4
- Clarify `CancelableOperation` docs HOT 4
- Inconsistent behavior of `Stream.listen` on broadcast streams HOT 1
- [Proposal] Add a CountDownLatch implementation
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from async.