Skip to content

Commit

Permalink
Added progress updates for all XMLHttpRequest upload types / fix cras…
Browse files Browse the repository at this point in the history
…h on closed connection

Summary:
This PR includes the same changes made in #16541, for addressing issues #11853/#15724. It adds upload progress updates for uploads with any request body type, and not just form-data.

Additionally, this PR also includes a commit for fixing an `IllegalStateException` when a user's connection gets closed or times out (issues #10423/#11016). Since this exception was occurring within the progress updates logic, it started being thrown more frequently as a result of adding progress updates to all uploads, which was why the original PR was reverted.

To test the upload progress updates, run the following JS to ensure events are now being dispatched:
```
const fileUri = 'file:///my_file.dat';
const url = 'http://my_post_url.com/';
const xhr = new XMLHttpRequest();

xhr.upload.onprogress = (event) => {
    console.log('progress: ' + event.loaded + ' / ' + event.total);
}

xhr.onreadystatechange = () => {if (xhr.readyState === 4) console.log('done');}

console.log('start');

xhr.open('POST', url);

// sending a file (wasn't sending progress)
xhr.setRequestHeader('Content-Type', 'image/jpeg');
xhr.send({ uri: fileUri });

// sending a string (wasn't sending progress)
xhr.setRequestHeader('Content-Type', 'text/plain');
xhr.send("some big string");

// sending form data (was already working)
xhr.setRequestHeader('Content-Type', 'multipart/form-data');
const formData = new FormData(); formData.append('test', 'data');
xhr.send(formData);
```

To test the crash fix:
In the RN Android project, before this change, set a breakpoint at `mRequestBody.writeTo(mBufferedSink);` of `ProgressRequestBody`, and wait a short while for a POST request with a non-null body to time out before resuming the app. Once resumed, if the connection was closed (the `closed` variable will be set to true in `RealBufferedSink`), an `IllegalStateException` will be thrown, which crashes the app. After the changes, an `IOException` will get thrown instead, which is already being properly handled.

As mentioned above, includes the same changes as #16541, with an additional commit.

[ANDROID] [BUGFIX] [XMLHttpRequest] - Added progress updates for all XMLHttpRequest upload types / fix crash on closed connection

Previously, only form-data request bodies emitted upload progress updates. Now, other request body types will also emit updates. Also, Android will no longer crash on certain requests when user has a poor connection.

Addresses issues: 11853/15724/10423/11016
Closes #17312

Differential Revision: D6712377

Pulled By: mdvacca

fbshipit-source-id: bf5adc774703e7e66f7f16707600116f67201425
  • Loading branch information
allengleyzer authored and facebook-github-bot committed Feb 3, 2018
1 parent ef596de commit 9e436d1
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ rn_android_library(
],
deps = [
react_native_dep("libraries/fbcore/src/main/java/com/facebook/common/logging:logging"),
react_native_dep("libraries/fbcore/src/main/java/com/facebook/common/internal:internal"),
react_native_dep("third-party/java/infer-annotations:infer-annotations"),
react_native_dep("third-party/java/jsr-305:jsr-305"),
react_native_dep("third-party/java/okhttp:okhttp3"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,11 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) {
}
}

RequestBody requestBody;
if (data == null) {
requestBuilder.method(method, RequestBodyUtil.getEmptyBody(method));
requestBody = RequestBodyUtil.getEmptyBody(method);
} else if (handler != null) {
RequestBody requestBody = handler.toRequestBody(data, contentType);
requestBuilder.method(method, requestBody);
requestBody = handler.toRequestBody(data, contentType);
} else if (data.hasKey(REQUEST_BODY_KEY_STRING)) {
if (contentType == null) {
ResponseUtil.onRequestError(
Expand All @@ -360,14 +360,13 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) {
String body = data.getString(REQUEST_BODY_KEY_STRING);
MediaType contentMediaType = MediaType.parse(contentType);
if (RequestBodyUtil.isGzipEncoding(contentEncoding)) {
RequestBody requestBody = RequestBodyUtil.createGzip(contentMediaType, body);
requestBody = RequestBodyUtil.createGzip(contentMediaType, body);
if (requestBody == null) {
ResponseUtil.onRequestError(eventEmitter, requestId, "Failed to gzip request body", null);
return;
}
requestBuilder.method(method, requestBody);
} else {
requestBuilder.method(method, RequestBody.create(contentMediaType, body));
requestBody = RequestBody.create(contentMediaType, body);
}
} else if (data.hasKey(REQUEST_BODY_KEY_BASE64)) {
if (contentType == null) {
Expand All @@ -380,9 +379,7 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) {
}
String base64String = data.getString(REQUEST_BODY_KEY_BASE64);
MediaType contentMediaType = MediaType.parse(contentType);
requestBuilder.method(
method,
RequestBody.create(contentMediaType, ByteString.decodeBase64(base64String)));
requestBody = RequestBody.create(contentMediaType, ByteString.decodeBase64(base64String));
} else if (data.hasKey(REQUEST_BODY_KEY_URI)) {
if (contentType == null) {
ResponseUtil.onRequestError(
Expand All @@ -403,9 +400,7 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) {
null);
return;
}
requestBuilder.method(
method,
RequestBodyUtil.create(MediaType.parse(contentType), fileInputStream));
requestBody = RequestBodyUtil.create(MediaType.parse(contentType), fileInputStream);
} else if (data.hasKey(REQUEST_BODY_KEY_FORMDATA)) {
if (contentType == null) {
contentType = "multipart/form-data";
Expand All @@ -416,28 +411,16 @@ public void onProgress(long bytesWritten, long contentLength, boolean done) {
if (multipartBuilder == null) {
return;
}

requestBuilder.method(
method,
RequestBodyUtil.createProgressRequest(
multipartBuilder.build(),
new ProgressListener() {
long last = System.nanoTime();

@Override
public void onProgress(long bytesWritten, long contentLength, boolean done) {
long now = System.nanoTime();
if (done || shouldDispatch(now, last)) {
ResponseUtil.onDataSend(eventEmitter, requestId, bytesWritten, contentLength);
last = now;
}
}
}));
requestBody = multipartBuilder.build();
} else {
// Nothing in data payload, at least nothing we could understand anyway.
requestBuilder.method(method, RequestBodyUtil.getEmptyBody(method));
requestBody = RequestBodyUtil.getEmptyBody(method);
}

requestBuilder.method(
method,
wrapRequestBodyWithProgressEmitter(requestBody, eventEmitter, requestId));

addRequest(requestId);
client.newCall(requestBuilder.build()).enqueue(
new Callback() {
Expand Down Expand Up @@ -515,6 +498,29 @@ public void onResponse(Call call, Response response) throws IOException {
});
}

private RequestBody wrapRequestBodyWithProgressEmitter(
final RequestBody requestBody,
final RCTDeviceEventEmitter eventEmitter,
final int requestId) {
if(requestBody == null) {
return null;
}
return RequestBodyUtil.createProgressRequest(
requestBody,
new ProgressListener() {
long last = System.nanoTime();

@Override
public void onProgress(long bytesWritten, long contentLength, boolean done) {
long now = System.nanoTime();
if (done || shouldDispatch(now, last)) {
ResponseUtil.onDataSend(eventEmitter, requestId, bytesWritten, contentLength);
last = now;
}
}
});
}

private void readWithProgress(
RCTDeviceEventEmitter eventEmitter,
int requestId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,60 +9,75 @@

package com.facebook.react.modules.network;

import com.facebook.common.internal.CountingOutputStream;

import java.io.IOException;

import okhttp3.MediaType;
import okhttp3.RequestBody;
import okio.BufferedSink;
import okio.Buffer;
import okio.Sink;
import okio.ForwardingSink;
import okio.Okio;
import okio.Sink;

public class ProgressRequestBody extends RequestBody {

private final RequestBody mRequestBody;
private final ProgressListener mProgressListener;
private BufferedSink mBufferedSink;
private long mContentLength = 0L;

public ProgressRequestBody(RequestBody requestBody, ProgressListener progressListener) {
mRequestBody = requestBody;
mProgressListener = progressListener;
mRequestBody = requestBody;
mProgressListener = progressListener;
}

@Override
public MediaType contentType() {
return mRequestBody.contentType();
return mRequestBody.contentType();
}

@Override
public long contentLength() throws IOException {
return mRequestBody.contentLength();
if (mContentLength == 0) {
mContentLength = mRequestBody.contentLength();
}
return mContentLength;
}

@Override
public void writeTo(BufferedSink sink) throws IOException {
if (mBufferedSink == null) {
mBufferedSink = Okio.buffer(sink(sink));
}
mRequestBody.writeTo(mBufferedSink);
mBufferedSink.flush();
if (mBufferedSink == null) {
mBufferedSink = Okio.buffer(outputStreamSink(sink));
}

// contentLength changes for input streams, since we're using inputStream.available(),
// so get the length before writing to the sink
contentLength();

mRequestBody.writeTo(mBufferedSink);
mBufferedSink.flush();
}

private Sink sink(Sink sink) {
return new ForwardingSink(sink) {
long bytesWritten = 0L;
long contentLength = 0L;
private Sink outputStreamSink(BufferedSink sink) {
return Okio.sink(new CountingOutputStream(sink.outputStream()) {
@Override
public void write(byte[] data, int offset, int byteCount) throws IOException {
super.write(data, offset, byteCount);
sendProgressUpdate();
}

@Override
public void write(Buffer source, long byteCount) throws IOException {
super.write(source, byteCount);
if (contentLength == 0) {
contentLength = contentLength();
}
bytesWritten += byteCount;
mProgressListener.onProgress(
bytesWritten, contentLength, bytesWritten == contentLength);
}
};
@Override
public void write(int data) throws IOException {
super.write(data);
sendProgressUpdate();
}

private void sendProgressUpdate() throws IOException {
long bytesWritten = getCount();
long contentLength = contentLength();
mProgressListener.onProgress(
bytesWritten, contentLength, bytesWritten == contentLength);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ public WritableMap answer(InvocationOnMock invocation) throws Throwable {

@Test
public void testSuccessfulPostRequest() throws Exception {
RCTDeviceEventEmitter emitter = mock(RCTDeviceEventEmitter.class);
ReactApplicationContext context = mock(ReactApplicationContext.class);
when(context.getJSModule(any(Class.class))).thenReturn(emitter);

OkHttpClient httpClient = mock(OkHttpClient.class);
when(httpClient.newCall(any(Request.class))).thenAnswer(new Answer<Object>() {
@Override
Expand All @@ -211,12 +215,13 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
OkHttpClient.Builder clientBuilder = mock(OkHttpClient.Builder.class);
when(clientBuilder.build()).thenReturn(httpClient);
when(httpClient.newBuilder()).thenReturn(clientBuilder);
NetworkingModule networkingModule =
new NetworkingModule(mock(ReactApplicationContext.class), "", httpClient);
NetworkingModule networkingModule = new NetworkingModule(context, "", httpClient);

JavaOnlyMap body = new JavaOnlyMap();
body.putString("string", "This is request body");

mockEvents();

networkingModule.sendRequest(
"POST",
"http://somedomain/bar",
Expand Down

0 comments on commit 9e436d1

Please sign in to comment.