Skip to content

Commit

Permalink
Fix for issue #468, #461, #460 and minor cleanup (#469)
Browse files Browse the repository at this point in the history 
* bump to 0.10.8

* Update PULL_REQUEST_TEMPLATE

* Fix #468 "Messy error returns: Sometimes a string, sometimes an Error object"

* Cleanup: remove an unused constant and duplicate method definitions

* Cleanup:
- fix minor errors in JSDoc comments, for example {string]} => {string}
- fix parameter name "encode" => "encoding" (more logical, and it says so in the function's JSDoc too)
- json-stream.js: split a looooong log message string constant into two parts and fix a typo ("maually"), and the type for objects is "Object" (capitalized) in Flow type annotations

* Fix a (Flow) type conflict - fixes issue #461

* NEEDS REVIEW - Attempt to fix some of issue #460 (Error message normalization)

Error messages reported by iOS and Android versions should be as similar as possible. Also, within the same system there should be consistency. This patch is an attempt to bring a LITTLE more of this consistency to the error messages. I also fixed some very few minor language issues, like "does not exist" (is the correct English). I tried keeping the changes to a minimum.

Background: In my project code I want to know when a file already exists (e.g. after calling fs.createFile), and the only way is to check the error message string that I get. It's bad if they differ between versions (createFileASCII and createFile) and then also between Android and iOS version. At least some core part of the string should be the same, so that I have something to match.

Ideally messages should come from a centralized easy (easier) to maintain file (for both iOS and Android), and ideally both systems should have the same errors and messages as far as possible.
  • Loading branch information
@LionessTester @wkh237
LionessTester authored and wkh237 committed Aug 9, 2017
1 parent 55009f1 commit b634402
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 89 deletions.
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Thank you for making a pull request ! Just a gentle reminder :)

1. If the PR is offering a feature please make the request to our "Feature Branch" 0.11.0
2. Bug fix request to "Bug Fix Branch" 0.10.8
2. Bug fix request to "Bug Fix Branch" 0.10.9
3. Correct README.md can directly to master
2 changes: 1 addition & 1 deletion android.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const RNFetchBlob:RNFetchBlobNative = NativeModules.RNFetchBlob

/**
* Send an intent to open the file.
* @param {string]} path Path of the file to be open.
* @param {string} path Path of the file to be open.
* @param {string} mime MIME type string
* @return {Promise}
*/
Expand Down
51 changes: 27 additions & 24 deletions android/src/main/java/com/RNFetchBlob/RNFetchBlobFS.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ static public void writeFile(String path, String encoding, String data, final bo
data = normalizePath(data);
File src = new File(data);
if(!src.exists()) {
promise.reject("RNfetchBlob writeFileError", "source file : " + data + "not exists");
promise.reject("RNfetchBlob writeFile error", "source file : " + data + " does not exist");
fout.close();
return ;
}
Expand All @@ -100,7 +100,7 @@ static public void writeFile(String path, String encoding, String data, final bo
fout.close();
promise.resolve(written);
} catch (Exception e) {
promise.reject("RNFetchBlob writeFileError", e.getLocalizedMessage());
promise.reject("RNFetchBlob writeFile error", e.getLocalizedMessage());
}
}

Expand All @@ -127,7 +127,7 @@ static public void writeFile(String path, ReadableArray data, final boolean appe
os.close();
promise.resolve(data.size());
} catch (Exception e) {
promise.reject("RNFetchBlob writeFileError", e.getLocalizedMessage());
promise.reject("RNFetchBlob writeFile error", e.getLocalizedMessage());
}
}

Expand Down Expand Up @@ -309,7 +309,8 @@ else if(resolved == null) {
buffer = null;

} catch (Exception err) {
emitStreamEvent(streamId, "warn", "Failed to convert data to "+encoding+" encoded string, this might due to the source data is not able to convert using this encoding.");
emitStreamEvent(streamId, "warn", "Failed to convert data to " + encoding +
" encoded string, this might due to the source data is not able to convert using this encoding.");
err.printStackTrace();
}
}
Expand All @@ -324,7 +325,7 @@ else if(resolved == null) {
public void writeStream(String path, String encoding, boolean append, Callback callback) {
File dest = new File(path);
if(!dest.exists() || dest.isDirectory()) {
callback.invoke("write stream error: target path `" + path + "` may not exists or it's a folder");
callback.invoke("target path `" + path + "` may not exist or it is a folder");
return;
}
try {
Expand All @@ -336,7 +337,7 @@ public void writeStream(String path, String encoding, boolean append, Callback c
this.writeStreamInstance = fs;
callback.invoke(null, streamId);
} catch(Exception err) {
callback.invoke("write stream error: failed to create write stream at path `"+path+"` "+ err.getLocalizedMessage());
callback.invoke("failed to create write stream at path `" + path + "` " + err.getLocalizedMessage());
}

}
Expand Down Expand Up @@ -433,12 +434,13 @@ static void deleteRecursive(File fileOrDirectory) {
static void mkdir(String path, Callback callback) {
File dest = new File(path);
if(dest.exists()) {
callback.invoke("mkdir error: failed to create folder at `" + path + "` folder already exists");
callback.invoke("mkdir failed, folder already exists at " + path);
return;
}
dest.mkdirs();
callback.invoke();
}

/**
* Copy file to destination path
* @param path Source path
Expand All @@ -454,7 +456,7 @@ static void cp(String path, String dest, Callback callback) {
try {

if(!isPathExists(path)) {
callback.invoke("cp error: source file at path`" + path + "` not exists");
callback.invoke("source file at path`" + path + "` does not exist");
return;
}
if(!new File(dest).exists())
Expand Down Expand Up @@ -495,7 +497,7 @@ static void cp(String path, String dest, Callback callback) {
static void mv(String path, String dest, Callback callback) {
File src = new File(path);
if(!src.exists()) {
callback.invoke("mv error: source file at path `" + path + "` does not exists");
callback.invoke("source file at path `" + path + "` does not exist");
return;
}
src.renameTo(new File(dest));
Expand Down Expand Up @@ -535,7 +537,7 @@ static void ls(String path, Callback callback) {
path = normalizePath(path);
File src = new File(path);
if (!src.exists() || !src.isDirectory()) {
callback.invoke("ls error: failed to list path `" + path + "` for it is not exist or it is not a folder");
callback.invoke("failed to list path `" + path + "` for it is not exist or it is not a folder");
return;
}
String[] files = new File(path).list();
Expand All @@ -559,7 +561,7 @@ public static void slice(String src, String dest, int start, int end, String enc
src = normalizePath(src);
File source = new File(src);
if(!source.exists()) {
promise.reject("RNFetchBlob.slice error", "source file : " + src + " not exists");
promise.reject("RNFetchBlob slice error", "source file : " + src + " does not exist");
return;
}
long size = source.length();
Expand All @@ -585,7 +587,7 @@ public static void slice(String src, String dest, int start, int end, String enc
promise.resolve(dest);
} catch (Exception e) {
e.printStackTrace();
promise.reject(e.getLocalizedMessage());
promise.reject("RNFetchBlob slice error", e.getLocalizedMessage());
}
}

Expand All @@ -597,18 +599,18 @@ static void lstat(String path, final Callback callback) {
protected Integer doInBackground(String ...args) {
WritableArray res = Arguments.createArray();
if(args[0] == null) {
callback.invoke("lstat error: the path specified for lstat is either `null` or `undefined`.");
callback.invoke("the path specified for lstat is either `null` or `undefined`.");
return 0;
}
File src = new File(args[0]);
if(!src.exists()) {
callback.invoke("lstat error: failed to list path `" + args[0] + "` for it is not exist or it is not a folder");
callback.invoke("failed to lstat path `" + args[0] + "` because it does not exist or it is not a folder");
return 0;
}
if(src.isDirectory()) {
String [] files = src.list();
for(String p : files) {
res.pushMap(statFile ( src.getPath() + "/" + p));
res.pushMap(statFile(src.getPath() + "/" + p));
}
}
else {
Expand All @@ -630,7 +632,7 @@ static void stat(String path, Callback callback) {
path = normalizePath(path);
WritableMap result = statFile(path);
if(result == null)
callback.invoke("stat error: failed to list path `" + path + "` for it is not exist or it is not a folder", null);
callback.invoke("failed to stat path `" + path + "` because it does not exist or it is not a folder", null);
else
callback.invoke(null, result);
} catch(Exception err) {
Expand Down Expand Up @@ -709,23 +711,24 @@ static void createFile(String path, String data, String encoding, Callback callb
String orgPath = data.replace(RNFetchBlobConst.FILE_PREFIX, "");
File src = new File(orgPath);
if(!src.exists()) {
callback.invoke("RNfetchBlob writeFileError", "source file : " + data + "not exists");
callback.invoke("source file : " + data + " does not exist");
return ;
}
FileInputStream fin = new FileInputStream(src);
OutputStream ostream = new FileOutputStream(dest);
byte [] buffer = new byte [10240];
byte[] buffer = new byte[10240];
int read = fin.read(buffer);
while(read > 0) {
while (read > 0) {
ostream.write(buffer, 0, read);
read = fin.read(buffer);
}
fin.close();
ostream.close();
}
else {
} else {
if (!created) {
callback.invoke("create file error: failed to create file at path `" + path + "` for its parent path may not exists, or the file already exists. If you intended to overwrite the existing file use fs.writeFile instead.");
callback.invoke("failed to create new file at path `" + path + "` because its parent path " +
"may not exist, or the file already exists. If you intended to overwrite the " +
"existing file use fs.writeFile instead.");
return;
}
OutputStream ostream = new FileOutputStream(dest);
Expand All @@ -747,12 +750,12 @@ static void createFileASCII(String path, ReadableArray data, Callback callback)
try {
File dest = new File(path);
if(dest.exists()) {
callback.invoke("create file error: failed to create file at path `" + path + "`, file already exists.");
callback.invoke("failed to create new file at path `" + path + "`, file already exists.");
return;
}
boolean created = dest.createNewFile();
if(!created) {
callback.invoke("create file error: failed to create file at path `" + path + "` for its parent path may not exists");
callback.invoke("failed to create new file at path `" + path + "` because its parent path may not exist");
return;
}
OutputStream ostream = new FileOutputStream(dest);
Expand Down
9 changes: 2 additions & 7 deletions class/RNFetchBlobSession.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,11 @@ import {
} from 'react-native'

const RNFetchBlob = NativeModules.RNFetchBlob
const emitter = DeviceEventEmitter

let sessions = {}

export default class RNFetchBlobSession {

add : (path:string) => RNFetchBlobSession;
remove : (path:string) => RNFetchBlobSession;
dispose : () => Promise;
list : () => Array<string>;
name : string;

static getSession(name:string):any {
Expand Down Expand Up @@ -50,7 +45,7 @@ export default class RNFetchBlobSession {

remove(path:string):RNFetchBlobSession {
let list = sessions[this.name]
for(let i in list) {
for(let i of list) {
if(list[i] === path) {
sessions[this.name].splice(i, 1)
break;
Expand All @@ -67,7 +62,7 @@ export default class RNFetchBlobSession {
return new Promise((resolve, reject) => {
RNFetchBlob.removeSession(sessions[this.name], (err) => {
if(err)
reject(err)
reject(new Error(err))
else {
delete sessions[this.name]
resolve()
Expand Down
13 changes: 6 additions & 7 deletions class/RNFetchBlobWriteStream.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ import {
} from 'react-native'

const RNFetchBlob = NativeModules.RNFetchBlob
const emitter = DeviceEventEmitter

export default class RNFetchBlobWriteStream {

id : string;
encoding : string;
append : bool;
append : boolean;

constructor(streamId:string, encoding:string, append:string) {
constructor(streamId:string, encoding:string, append:boolean) {
this.id = streamId
this.encoding = encoding
this.append = append
Expand All @@ -28,17 +27,17 @@ export default class RNFetchBlobWriteStream {
try {
let method = this.encoding === 'ascii' ? 'writeArrayChunk' : 'writeChunk'
if(this.encoding.toLocaleLowerCase() === 'ascii' && !Array.isArray(data)) {
reject('ascii input data must be an Array')
reject(new Error('ascii input data must be an Array'))
return
}
RNFetchBlob[method](this.id, data, (error) => {
if(error)
reject(error)
reject(new Error(error))
else
resolve()
})
} catch(err) {
reject(err)
reject(new Error(err))
}
})
}
Expand All @@ -50,7 +49,7 @@ export default class RNFetchBlobWriteStream {
resolve()
})
} catch (err) {
reject(err)
reject(new Error(err))
}
})
}
Expand Down
24 changes: 12 additions & 12 deletions fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type {
} from './types'

const RNFetchBlob:RNFetchBlobNative = NativeModules.RNFetchBlob
const emitter = DeviceEventEmitter

const dirs = {
DocumentDir : RNFetchBlob.DocumentDir,
CacheDir : RNFetchBlob.CacheDir,
Expand Down Expand Up @@ -83,13 +83,13 @@ function createFile(path:string, data:string, encoding: 'base64' | 'ascii' | 'ut
* Create write stream to a file.
* @param {string} path Target path of file stream.
* @param {string} encoding Encoding of input data.
* @param {bool} append A flag represent if data append to existing ones.
* @return {Promise<WriteStream>} A promise resolves a `WriteStream` object.
* @param {boolean} [append] A flag represent if data append to existing ones.
* @return {Promise<RNFetchBlobWriteStream>} A promise resolves a `WriteStream` object.
*/
function writeStream(
path : string,
encoding : 'utf8' | 'ascii' | 'base64',
append? : ?bool,
append? : ?boolean,
):Promise<RNFetchBlobWriteStream> {
if(!path)
throw Error('RNFetchBlob could not open file stream with empty `path`')
Expand All @@ -110,6 +110,7 @@ function writeStream(
* @param {string} path The file path.
* @param {string} encoding Data encoding, should be one of `base64`, `utf8`, `ascii`
* @param {boolean} bufferSize Size of stream buffer.
* @param {number} [tick=10] Interval in milliseconds between reading chunks of data
* @return {RNFetchBlobStream} RNFetchBlobStream stream instance.
*/
function readStream(
Expand Down Expand Up @@ -154,7 +155,7 @@ function pathForAppGroup(groupName:string):Promise {
* @param {'base64' | 'utf8' | 'ascii'} encoding Encoding of read stream.
* @return {Promise<Array<number> | string>}
*/
function readFile(path:string, encoding:string, bufferSize:?number):Promise<any> {
function readFile(path:string, encoding:string):Promise<any> {
if(typeof path !== 'string')
return Promise.reject(new Error('Invalid argument "path" '))
return RNFetchBlob.readFile(path, encoding)
Expand All @@ -170,7 +171,7 @@ function readFile(path:string, encoding:string, bufferSize:?number):Promise<any>
function writeFile(path:string, data:string | Array<number>, encoding:?string):Promise {
encoding = encoding || 'utf8'
if(typeof path !== 'string')
return Promise.reject('Invalid argument "path" ')
return Promise.reject(new Error('Invalid argument "path" '))
if(encoding.toLocaleLowerCase() === 'ascii') {
if(!Array.isArray(data))
return Promise.reject(new Error(`Expected "data" is an Array when encoding is "ascii", however got ${typeof data}`))
Expand All @@ -187,7 +188,7 @@ function writeFile(path:string, data:string | Array<number>, encoding:?string):P
function appendFile(path:string, data:string | Array<number>, encoding:?string):Promise {
encoding = encoding || 'utf8'
if(typeof path !== 'string')
return Promise.reject('Invalid argument "path" ')
return Promise.reject(new Error('Invalid argument "path" '))
if(encoding.toLocaleLowerCase() === 'ascii') {
if(!Array.isArray(data))
return Promise.reject(new Error(`Expected "data" is an Array when encoding is "ascii", however got ${typeof data}`))
Expand Down Expand Up @@ -224,7 +225,7 @@ function stat(path:string):Promise<RNFetchBlobFile> {

/**
* Android only method, request media scanner to scan the file.
* @param {Array<Object<string, string>>} Array contains Key value pairs with key `path` and `mime`.
* @param {Array<Object<string, string>>} pairs Array contains Key value pairs with key `path` and `mime`.
* @return {Promise}
*/
function scanFile(pairs:any):Promise {
Expand Down Expand Up @@ -302,10 +303,9 @@ function unlink(path:string):Promise {
/**
* Check if file exists and if it is a folder.
* @param {string} path Path to check
* @return {Promise<bool, bool>}
* @return {Promise<boolean, boolean>}
*/
function exists(path:string):Promise<bool, bool> {

function exists(path:string):Promise<boolean, boolean> {
return new Promise((resolve, reject) => {
try {
RNFetchBlob.exists(path, (exist) => {
Expand Down Expand Up @@ -358,7 +358,7 @@ function df():Promise<{ free : number, total : number }> {
return new Promise((resolve, reject) => {
RNFetchBlob.df((err, stat) => {
if(err)
reject(err)
reject(new Error(err))
else
resolve(stat)
})
Expand Down
Loading

0 comments on commit b634402

Please sign in to comment.