Opened 5 years ago

Closed 5 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 5 years ago by dylan

Component: GeneralIO
Milestone: tbd1.11
Owner: set to Bryan Forbes
Priority: undecidedhigh
Status: newassigned

comment:2 Changed 5 years ago by dylan

Milestone: 1.111.10
Summary: Add support for onProgress event when using Chunked Transfer Encoding[patch][cla] Add support for onProgress event when using Chunked Transfer Encoding

Very trivial change, will consider for 1.10

comment:4 Changed 5 years ago by dylans <dylan@…>

Resolution: fixed
Status: assignedclosed

In e4dea97e56cdf3891df165d0b9a73d7848f5f137/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:5 Changed 5 years ago by dylan

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 5 years ago by hwinkler

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

Last edited 5 years ago by hwinkler (previous) (diff)

comment:7 Changed 5 years ago by bill

Resolution: fixed
Status: closedreopened

comment:8 Changed 5 years ago by hwinkler

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 5 years ago by hwinkler

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 5 years ago by dylan

Owner: changed from Bryan Forbes to dylan
Priority: highblocker
Status: reopenedassigned

comment:12 Changed 5 years ago by dylan

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.