Opened 7 years ago
Closed 7 years ago
#17823 closed enhancement (fixed)
[patch][cla] Add support for onProgress event when using Chunked Transfer Encoding
Reported by: | lsolano | Owned by: | dylan |
---|---|---|---|
Priority: | blocker | Milestone: | 1.10 |
Component: | IO | Version: | 1.9.3 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
Modify the dojo/request/xhr
wrapper to consider a progress event when the Transfer-Encoding
header value is chunked
and the readyState
is 3 (LOADING)
.
Change History (12)
comment:1 Changed 7 years ago by
Component: | General → IO |
---|---|
Milestone: | tbd → 1.11 |
Owner: | set to Bryan Forbes |
Priority: | undecided → high |
Status: | new → assigned |
comment:2 Changed 7 years ago by
Milestone: | 1.11 → 1.10 |
---|---|
Summary: | Add support for onProgress event when using Chunked Transfer Encoding → [patch][cla] Add support for onProgress event when using Chunked Transfer Encoding |
comment:4 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:5 Changed 7 years ago by
Note for posterity: I reviewed the background information on chunked encoding and agree with the approach in this patch and committed it. I would like to add a test for this, but it's not easy for us to test without setting up a server that will returned chunked data back to us, so I have not added a test at this time.
comment:6 Changed 7 years ago by
This change causes Chrome to throw an exception on every CORS request that emits an onProgress event:
'Refused to get unsafe header "Transfer-Encoding"'
Transfer-Encoding is not a "simple response header" as described in http://www.w3.org/TR/cors/. Search for "simple response header".
In my case, even though I control the server, even setting the Access-Control-Expose-Headers header in the response, to name Transfer-Encoding, does not alleviate the problem. Chrome still complains loudly and throws an exception.
This is dojo 1.10.0-beta1, Chrome 35.0.1916.86 beta
comment:7 Changed 7 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:8 Changed 7 years ago by
The thrown exception does not interfere with the completion of the requests. They complete normally.
The simplest approach would be to trap the exception. In that case we should probably move the if readyState === 3 outside and nest the if Transfer-Encoding test inside that.
It would also be possible to compare the current dojo.global.location.origin against the origin derived from response.url.
comment:9 Changed 7 years ago by
Chrome sets evt.position, even though it won't allow you to read the Transfer-Encoding header.
I started to do the followings, then realized, duh.
function onProgress(evt){ if(evt.lengthComputable){ response.loaded = evt.loaded; response.total = evt.total; dfd.progress(response); } else if(response.xhr.readyState === 3) { try { if (response.xhr.getResponseHeader('Transfer-Encoding') === 'chunked'){ response.loaded = evt.position; dfd.progress(response); } } catch(e) { response.loaded = evt.position; // hm.... why even try/catch? dfd.progress(response); } } }
So: do we really have to check for Transfer-Encoding === 'chunked' before using evt.position? Would the following be wrong?
function onProgress(evt){ if(evt.lengthComputable){ response.loaded = evt.loaded; response.total = evt.total; dfd.progress(response); } else if(response.xhr.readyState === 3) { response.loaded = evt.position; dfd.progress(response); } }
comment:10 Changed 7 years ago by
Owner: | changed from Bryan Forbes to dylan |
---|---|
Priority: | high → blocker |
Status: | reopened → assigned |
comment:12 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Very trivial change, will consider for 1.10