yutakahirano / fetch-with-streams

Fetch API integrated with Streams.
66 stars 8 forks source link

bodyUsed and opacity #61

Closed yutakahirano closed 6 years ago

yutakahirano commented 8 years ago

bodyUsed returns the IsDisturbed property value of the associated readable stream. It sounds natural to me that bodyUsed returns true when the internal response is disturbed. (This is not yet specified. I am planning to describe it in part of #40.)

var req = ...;
// Assume |res| is a filtered response having a non-null opaque body.
var res = ...;
assert_equals(null, res.body);
assert_false(res.bodyUsed);
var p = cache.put(req, res);
assert_true(res.bodyUsed);

p.then(() => {
  assert_equals(null, res.body);
  var q = res.text();
  // |q| should be rejected.
});

On the other hand, text() uses the usual body. That means calling text() does not set bodyUsed in this case because the associated readable stream is null.

var req = ...;
// Assume |res| is a filtered response having a non-null opaque body.
var res = ...;
assert_false(res.bodyUsed);
assert_equals(null, res.body);
var p = res.text();
assert_false(res.bodyUsed);
p.then(() => {
  assert_false(res.bodyUsed);
  var q = cache.put(req, res);
  assert_true(res.bodyUsed);
  // |q| should be fulfilled (unless there is another error).
});

In short, you can call text() and then call cache.put(). You cannot call them in reverse order. Is it reasonable?

wanderview commented 8 years ago

In short, you can call text() and then call cache.put(). You cannot call them in reverse order. Is it reasonable?

This is only true for opaque streams?

Can we make .text() and friends ignore the inner response's IsDisturbed() for opaque filtered responses? That way at least text() and cache.put() would act consistently regardless of call order. They would still react a little bit differently, though. .text() would never reject for bodyUsed, while cache.put() might.

yutakahirano commented 8 years ago

This is only true for opaque streams?

For non-filtered response with a non-null body, the first operation succeeds and the second fails.

Can we make .text() and friends ignore the inner response's IsDisturbed() for opaque filtered responses?

It is technically possible, doesn't it make bodyUsed unclearer than now? We will want to another bodyUsed property, perhaps.

wanderview commented 8 years ago

For non-filtered response with a non-null body, the first operation succeeds and the second fails.

Wait, what? I thought we had this working consistently between .text() and cache.put() for non-null body. These should absolutely work the same as they are both consuming the stream.

yutakahirano commented 8 years ago

When we support opaque streams (see #56), we may define IsDisturbed on it. That means calling text() sets bodyUsed for such responses. Do you like it?

I just noticed https://w3c.github.io/webappsec-credential-management/#monkey-patching-fetch-2. Perhaps we need opaque streams for Request, too...

yutakahirano commented 8 years ago

For non-filtered response with a non-null body, the first operation succeeds and the second fails.

Wait, what? I thought we had this working consistently between .text() and cache.put() for non-null body. These should absolutely work the same as they are both consuming the stream.

I think my expression was bad. I meant that regardless of the order the first one operation executed formerly succeeds and the second one other fails. When text() precedes, text() succeeds and cache.put fails. When cache.put() precedes, cache.put succeeds and text() fails.

wanderview commented 8 years ago

When we support opaque streams (see #56), we may define IsDisturbed on it. That means calling text() sets bodyUsed for such responses. Do you like it?

I guess this works. Maybe it lets script observe if the underlying opaque stream is null or not, though? Not sure that matters, though, since it could still be an empty string stream.