Skip to content

Commit

Permalink
Fix released readers to return rejected promises
Browse files Browse the repository at this point in the history
Fixes @yutakahirano's comment at #251 (comment).
  • Loading branch information
domenic committed Dec 11, 2014
1 parent 12374d7 commit bbe37d8
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 6 deletions.
29 changes: 23 additions & 6 deletions reference-implementation/lib/exclusive-stream-reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ export default class ExclusiveStreamReader {
}

get ready() {
ensureStreamReaderIsExclusive(this);
if (this._stream === undefined) {
return Promise.reject(isNotLockedError());
}
assertStreamReaderRelationshipIsCorrect(this);

this._stream._reader = undefined;
try {
Expand All @@ -40,7 +43,10 @@ export default class ExclusiveStreamReader {
}

get closed() {
ensureStreamReaderIsExclusive(this);
if (this._stream === undefined) {
return Promise.reject(isNotLockedError());
}
assertStreamReaderRelationshipIsCorrect(this);

this._stream._reader = undefined;
try {
Expand All @@ -66,7 +72,10 @@ export default class ExclusiveStreamReader {
}

cancel(reason, ...args) {
ensureStreamReaderIsExclusive(this);
if (this._stream === undefined) {
return Promise.reject(isNotLockedError());
}
assertStreamReaderRelationshipIsCorrect(this);

var stream = this._stream;
this.releaseLock();
Expand Down Expand Up @@ -94,17 +103,25 @@ export default class ExclusiveStreamReader {
// factor them out into helper functions in the reference implementation just for brevity's sake, and to emphasize that
// the error message is the same in all places they're called, and to give us the opportunity to add an assert.

function assertStreamReaderRelationshipIsCorrect(reader) {
assert(reader._stream._reader === reader,
'If the reader has a [[stream]] then the stream\'s [[reader]] must be this reader');
}

function ensureStreamReaderIsExclusive(reader) {
if (reader._stream === undefined) {
throw new TypeError('This stream reader has released its lock on the stream and can no longer be used');
throw isNotLockedError();
}

assert(reader._stream._reader === reader,
'If the reader has a [[stream]] then the stream\'s [[reader]] must be this reader');
assertStreamReaderRelationshipIsCorrect(reader);
}

function ensureIsRealStream(stream) {
if (!('_reader' in stream)) {
throw new TypeError('ExclusiveStreamReader can only be used with ReadableStream objects or subclasses');
}
}

function isNotLockedError() {
return new TypeError('This stream reader has released its lock on the stream and can no longer be used');
}
32 changes: 32 additions & 0 deletions reference-implementation/test/exclusive-stream-reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,35 @@ test('Using the reader directly on a mundane stream', t => {
t.equal(rs.state, 'closed', 'the stream state is closed');
});
});

test('Trying to use a released reader', t => {
t.plan(6);

var rs = new ReadableStream({
start(enqueue) {
enqueue('a');
enqueue('b');
}
});
var reader = rs.getReader();
reader.releaseLock();

t.equal(reader.isActive, false, 'isActive returns false');
t.throws(() => reader.state, /TypeError/, 'trying to get reader.state gives a TypeError');
t.throws(() => reader.read(), /TypeError/, 'trying to read gives a TypeError');

reader.ready.then(
() => t.fail('ready should not be fulfilled'),
e => t.equal(e.constructor, TypeError, 'ready should be rejected with a TypeError')
);

reader.closed.then(
() => t.fail('closed should not be fulfilled'),
e => t.equal(e.constructor, TypeError, 'closed should be rejected with a TypeError')
);

reader.cancel().then(
() => t.fail('cancel() should not be fulfilled'),
e => t.equal(e.constructor, TypeError, 'cancel() should be rejected with a TypeError')
);
});

0 comments on commit bbe37d8

Please sign in to comment.