Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

uploadContent: Attempt some consistency between browser and node #230

Merged
merged 2 commits into from
Oct 10, 2016
Merged

uploadContent: Attempt some consistency between browser and node #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d505ab9
Fix error handling on uploadContent
richvdh Oct 9, 2016
4794dfc
uploadContent: Attempt some consistency between browser and node
richvdh Oct 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions lib/base-apis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,13 +574,38 @@ MatrixBaseApis.prototype.setRoomDirectoryVisibility =

/**
* Upload a file to the media repository on the home server.
* @param {File} file object
* @param {module:client.callback} callback Optional.
* @return {module:client.Promise} Resolves: TODO
* @return {module:http-api.MatrixError} Rejects: with an error response.
*
* @param {object} file The object to upload. On a browser, something that
* can be sent to XMLHttpRequest.send (typically a File). Under node.js,
* a a Buffer, String or ReadStream.
*
* @param {object} opts options object
*
* @param {string=} opts.name Name to give the file on the server. Defaults
* to <tt>file.name</tt>.
*
* @param {string=} opts.type Content-type for the upload. Defaults to
* <tt>file.type</tt>, or <tt>applicaton/octet-stream</tt>.
*
* @param {boolean=} opts.rawResponse Return the raw body, rather than
* parsing the JSON. Defaults to false (except on node.js, where it
* defaults to true for backwards compatibility).
*
* @param {boolean=} opts.onlyContentUri Just return the content URI,
* rather than the whole body. Defaults to false (except on browsers,
* where it defaults to true for backwards compatibility). Ignored if
* opts.rawResponse is true.
*
* @param {Function=} opts.callback Deprecated. Optional. The callback to
* invoke on success/failure. See the promise return values for more
* information.
*
* @return {module:client.Promise} Resolves to response object, as
* determined by this.opts.onlyData, opts.rawResponse, and
* opts.onlyContentUri. Rejects with an error (usually a MatrixError).
*/
MatrixBaseApis.prototype.uploadContent = function(file, callback) {
return this._http.uploadContent(file, callback);
MatrixBaseApis.prototype.uploadContent = function(file, opts) {
return this._http.uploadContent(file, opts);
};

/**
Expand Down
233 changes: 186 additions & 47 deletions lib/http-api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ module.exports.PREFIX_MEDIA_R0 = "/_matrix/media/r0";
* requests. This function must look like function(opts, callback){ ... }.
* @param {string} opts.prefix Required. The matrix client prefix to use, e.g.
* '/_matrix/client/r0'. See PREFIX_R0 and PREFIX_UNSTABLE for constants.
* @param {bool} opts.onlyData True to return only the 'data' component of the
* response (e.g. the parsed HTTP body). If false, requests will return status
* codes and headers in addition to data. Default: false.
*
* @param {bool=} opts.onlyData True to return only the 'data' component of the
* response (e.g. the parsed HTTP body). If false, requests will return an
* object with the properties <tt>code</tt>, <tt>headers</tt> and <tt>data</tt>.
*
* @param {string} opts.accessToken The access_token to send with requests. Can be
* null to not send an access token.
* @param {Object} opts.extraParams Optional. Extra query parameters to send on
Expand Down Expand Up @@ -99,20 +101,87 @@ module.exports.MatrixHttpApi.prototype = {

/**
* Upload content to the Home Server
* @param {File} file A File object (in a browser) or in Node,
an object with properties:
name: The file's name
stream: A read stream
* @param {Function} callback Optional. The callback to invoke on
* success/failure. See the promise return values for more information.
* @return {module:client.Promise} Resolves to <code>{data: {Object},
*
* @param {object} file The object to upload. On a browser, something that
* can be sent to XMLHttpRequest.send (typically a File). Under node.js,
* a a Buffer, String or ReadStream.
*
* @param {object} opts options object
*
* @param {string=} opts.name Name to give the file on the server. Defaults
* to <tt>file.name</tt>.
*
* @param {string=} opts.type Content-type for the upload. Defaults to
* <tt>file.type</tt>, or <tt>applicaton/octet-stream</tt>.
*
* @param {boolean=} opts.rawResponse Return the raw body, rather than
* parsing the JSON. Defaults to false (except on node.js, where it
* defaults to true for backwards compatibility).
*
* @param {boolean=} opts.onlyContentUri Just return the content URI,
* rather than the whole body. Defaults to false (except on browsers,
* where it defaults to true for backwards compatibility). Ignored if
* opts.rawResponse is true.
*
* @param {Function=} opts.callback Deprecated. Optional. The callback to
* invoke on success/failure. See the promise return values for more
* information.
*
* @return {module:client.Promise} Resolves to response object, as
* determined by this.opts.onlyData, opts.rawResponse, and
* opts.onlyContentUri. Rejects with an error (usually a MatrixError).
*/
uploadContent: function(file, callback) {
if (callback !== undefined && !utils.isFunction(callback)) {
throw Error(
"Expected callback to be a function but got " + typeof callback
);
uploadContent: function(file, opts) {
if (utils.isFunction(opts)) {
// opts used to be callback
opts = {
callback: opts,
};
} else if (opts === undefined) {
opts = {};
}

// if the file doesn't have a mime type, use a default since
// the HS errors if we don't supply one.
var contentType = opts.type || file.type || 'application/octet-stream';
var fileName = opts.name || file.name;

// we used to recommend setting file.stream to the thing to upload on
// nodejs.
var body = file.stream ? file.stream : file;

// backwards-compatibility hacks where we used to do different things
// between browser and node.
var rawResponse = opts.rawResponse;
if (rawResponse === undefined) {
if (global.XMLHttpRequest) {
rawResponse = false;
} else {
console.warn(
"Returning the raw JSON from uploadContent(). Future " +
"versions of the js-sdk will change this default, to " +
"return the parsed object. Set opts.rawResponse=false " +
"to change this behaviour now."
);
rawResponse = true;
}
}

var onlyContentUri = opts.onlyContentUri;
if (!rawResponse && onlyContentUri === undefined) {
if (global.XMLHttpRequest) {
console.warn(
"Returning only the content-uri from uploadContent(). " +
"Future versions of the js-sdk will change this " +
"default, to return the whole response object. Set " +
"opts.onlyContentUri=false to change this behaviour now."
);
onlyContentUri = true;
} else {
onlyContentUri = false;
}
}

// browser-request doesn't support File objects because it deep-copies
// the options using JSON.parse(JSON.stringify(options)). Instead of
// loading the whole file into memory as a string and letting
Expand All @@ -123,11 +192,30 @@ module.exports.MatrixHttpApi.prototype = {

var upload = { loaded: 0, total: 0 };
var promise;

// XMLHttpRequest doesn't parse JSON for us. request normally does, but
// we're setting opts.json=false so that it doesn't JSON-encode the
// request, which also means it doesn't JSON-decode the response. Either
// way, we have to JSON-parse the response ourselves.
var bodyParser = null;
if (!rawResponse) {
bodyParser = function(rawBody) {
var body = JSON.parse(rawBody);
if (onlyContentUri) {
body = body.content_uri;
if (body === undefined) {
throw Error('Bad response');
}
}
return body;
};
}

if (global.XMLHttpRequest) {
var defer = q.defer();
var xhr = new global.XMLHttpRequest();
upload.xhr = xhr;
var cb = requestCallback(defer, callback, this.opts.onlyData);
var cb = requestCallback(defer, opts.callback, this.opts.onlyData);

var timeout_fn = function() {
xhr.abort();
Expand All @@ -142,23 +230,21 @@ module.exports.MatrixHttpApi.prototype = {
switch (xhr.readyState) {
case global.XMLHttpRequest.DONE:
callbacks.clearTimeout(xhr.timeout_timer);
var err;
if (!xhr.responseText) {
err = new Error('No response body.');
var resp;
try {
if (!xhr.responseText) {
throw new Error('No response body.');
}
resp = xhr.responseText;
if (bodyParser) {
resp = bodyParser(resp);
}
} catch (err) {
err.http_status = xhr.status;
cb(err);
return;
}

var resp = JSON.parse(xhr.responseText);
if (resp.content_uri === undefined) {
err = Error('Bad response');
err.http_status = xhr.status;
cb(err);
return;
}

cb(undefined, xhr, resp.content_uri);
cb(undefined, xhr, resp);
break;
}
};
Expand All @@ -171,30 +257,26 @@ module.exports.MatrixHttpApi.prototype = {
});
var url = this.opts.baseUrl + "/_matrix/media/v1/upload";
url += "?access_token=" + encodeURIComponent(this.opts.accessToken);
url += "&filename=" + encodeURIComponent(file.name);
url += "&filename=" + encodeURIComponent(fileName);

xhr.open("POST", url);
if (file.type) {
xhr.setRequestHeader("Content-Type", file.type);
} else {
// if the file doesn't have a mime type, use a default since
// the HS errors if we don't supply one.
xhr.setRequestHeader("Content-Type", 'application/octet-stream');
}
xhr.send(file);
xhr.setRequestHeader("Content-Type", contentType);
xhr.send(body);
promise = defer.promise;

// dirty hack (as per _request) to allow the upload to be cancelled.
promise.abort = xhr.abort.bind(xhr);
} else {
var queryParams = {
filename: file.name,
filename: fileName,
};

promise = this.authedRequest(
callback, "POST", "/upload", queryParams, file.stream, {
opts.callback, "POST", "/upload", queryParams, body, {
prefix: "/_matrix/media/v1",
headers: {"Content-Type": file.type},
headers: {"Content-Type": contentType},
json: false,
bodyParser: bodyParser,
}
);
}
Expand Down Expand Up @@ -495,6 +577,32 @@ module.exports.MatrixHttpApi.prototype = {
return this.opts.baseUrl + prefix + path + queryString;
},

/**
* @private
*
* @param {function} callback
* @param {string} method
* @param {string} uri
* @param {object} queryParams
* @param {object|string} data
* @param {object=} opts
*
* @param {boolean=true} opts.json Json-encode data before sending, and
* decode response on receipt. (We will still json-decode error
* responses, even if this is false.)
*
* @param {object=} opts.headers extra request headers
*
* @param {number=} opts.localTimeoutMs client-side timeout for the
* request. No timeout if undefined.
*
* @param {function=} opts.bodyParser function to parse the body of the
* response before passing it to the promise and callback.
*
* @return {module:client.Promise} a promise which resolves to either the
* response object (if this.opts.onlyData is truthy), or the parsed
* body. Rejects
*/
_request: function(callback, method, uri, queryParams, data, opts) {
if (callback !== undefined && !utils.isFunction(callback)) {
throw Error(
Expand All @@ -510,6 +618,9 @@ module.exports.MatrixHttpApi.prototype = {
queryParams[key] = this.opts.extraParams[key];
}
}

var json = opts.json === undefined ? true : opts.json;

var defer = q.defer();

var timeoutId;
Expand Down Expand Up @@ -540,7 +651,7 @@ module.exports.MatrixHttpApi.prototype = {
withCredentials: false,
qs: queryParams,
body: data,
json: opts.json === undefined ? true : opts.json,
json: json,
timeout: localTimeoutMs,
headers: opts.headers || {},
_matrix_opts: this.opts
Expand All @@ -552,7 +663,15 @@ module.exports.MatrixHttpApi.prototype = {
return; // already rejected promise
}
}
var handlerFn = requestCallback(defer, callback, self.opts.onlyData);

// if json is falsy, we won't parse any error response, so need
// to do so before turning it into a MatrixError
var parseErrorJson = !json;
var handlerFn = requestCallback(
defer, callback, self.opts.onlyData,
parseErrorJson,
opts.bodyParser
);
handlerFn(err, response, body);
}
);
Expand All @@ -579,14 +698,34 @@ module.exports.MatrixHttpApi.prototype = {
*
* If onlyData is true, the defer/callback is invoked with the body of the
* response, otherwise the result code.
*
* If parseErrorJson is true, we will JSON.parse the body if we get a 4xx error.
*
*/
var requestCallback = function(defer, userDefinedCallback, onlyData) {
var requestCallback = function(
defer, userDefinedCallback, onlyData,
parseErrorJson, bodyParser
) {
userDefinedCallback = userDefinedCallback || function() {};

return function(err, response, body) {
if (!err && response.statusCode >= 400) {
err = new module.exports.MatrixError(body);
err.httpStatus = response.statusCode;
if (!err) {
try {
if (response.statusCode >= 400) {
if (parseErrorJson) {
// we won't have json-decoded the response.
body = JSON.parse(body);
}
err = new module.exports.MatrixError(body);
} else if (bodyParser) {
body = bodyParser(body);
}
} catch (e) {
err = e;
}
if (err) {
err.httpStatus = response.statusCode;
}
}

if (err) {
Expand Down
Loading