Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Suggestion: Return error objects instead of error message string #1479

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 27 additions & 27 deletions src/storage/storage.android.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function getReference(nativeReference: com.google.firebase.storage.StorageRefere
bucket: nativeReference.getBucket(),
name: nativeReference.getName(),
fullPath: nativeReference.getPath(),
listAll: (): Promise<ListResult> => listAll({remoteFullPath: nativeReference.getPath(), bucket: listOptions.bucket})
listAll: (): Promise<ListResult> => listAll({ remoteFullPath: nativeReference.getPath(), bucket: listOptions.bucket })
}
}

Expand Down Expand Up @@ -56,7 +56,7 @@ function getStorageRef(reject, arg) {
}
}

export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {
export function uploadFile(arg: UploadFileOptions): Promise<any> {
return new Promise((resolve, reject) => {
try {

Expand Down Expand Up @@ -85,7 +85,7 @@ export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {

const onFailureListener = new gmsTasks.OnFailureListener({
onFailure: exception => {
reject("Upload failed. " + exception);
reject(exception);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's wise to return the native Java Exception object here. It used to be a String representation. Not saying that's perfect, but this is not better imo.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure either, that's why it's a rough fix. But if not the entire object, the Google error code may still work.

}
});

Expand All @@ -110,9 +110,9 @@ export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {
// using 'putFile' (not 'putBytes') so Firebase can infer the mimetype
const localFileUrl = android.net.Uri.fromFile(new java.io.File(arg.localFile.path));
storageReference.putFile(localFileUrl)
.addOnFailureListener(onFailureListener)
.addOnSuccessListener(onSuccessListener)
.addOnProgressListener(onProgressListener);
.addOnFailureListener(onFailureListener)
.addOnSuccessListener(onSuccessListener)
.addOnProgressListener(onProgressListener);

/*
var error;
Expand All @@ -138,9 +138,9 @@ export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {

const localFileUrl = android.net.Uri.fromFile(new java.io.File(arg.localFullPath));
storageReference.putFile(localFileUrl)
.addOnFailureListener(onFailureListener)
.addOnSuccessListener(onSuccessListener)
.addOnProgressListener(onProgressListener);
.addOnFailureListener(onFailureListener)
.addOnSuccessListener(onSuccessListener)
.addOnProgressListener(onProgressListener);

} else {
reject("One of localFile or localFullPath is required");
Expand All @@ -153,7 +153,7 @@ export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {
});
}

export function downloadFile(arg: DownloadFileOptions): Promise<string> {
export function downloadFile(arg: DownloadFileOptions): Promise<any> {
return new Promise((resolve, reject) => {
try {

Expand All @@ -170,7 +170,7 @@ export function downloadFile(arg: DownloadFileOptions): Promise<string> {
});

const onFailureListener = new gmsTasks.OnFailureListener({
onFailure: exception => reject("Download failed. " + exception)
onFailure: exception => reject(exception)
});

let localFilePath;
Expand All @@ -193,8 +193,8 @@ export function downloadFile(arg: DownloadFileOptions): Promise<string> {
const file = new java.io.File(localFilePath);

storageReference.getFile(file)
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);

} catch (ex) {
console.log("Error in firebase.downloadFile: " + ex);
Expand All @@ -203,7 +203,7 @@ export function downloadFile(arg: DownloadFileOptions): Promise<string> {
});
}

export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<string> {
export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<any> {
return new Promise((resolve, reject) => {
try {

Expand All @@ -223,13 +223,13 @@ export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<string> {

const onFailureListener = new gmsTasks.OnFailureListener({
onFailure: exception => {
reject(exception.getMessage());
reject(exception);
}
});

storageReference.getDownloadUrl()
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);

} catch (ex) {
console.log("Error in firebase.getDownloadUrl: " + ex);
Expand All @@ -238,8 +238,8 @@ export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<string> {
});
}

export function deleteFile(arg: DeleteFileOptions): Promise<void> {
return new Promise<void>((resolve, reject) => {
export function deleteFile(arg: DeleteFileOptions): Promise<any> {
return new Promise<any>((resolve, reject) => {
try {

const storageRef = getStorageRef(reject, arg);
Expand All @@ -258,13 +258,13 @@ export function deleteFile(arg: DeleteFileOptions): Promise<void> {

const onFailureListener = new gmsTasks.OnFailureListener({
onFailure: exception => {
reject(exception.getMessage());
reject(exception);
}
});

storageReference.delete()
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);

} catch (ex) {
console.log("Error in firebase.deleteFile: " + ex);
Expand All @@ -273,8 +273,8 @@ export function deleteFile(arg: DeleteFileOptions): Promise<void> {
});
}

export function listAll(listOptions: ListOptions): Promise<ListResult> {
return new Promise<ListResult>((resolve, reject) => {
export function listAll(listOptions: ListOptions): Promise<any> {
return new Promise<any>((resolve, reject) => {
try {
const storageRef = getStorageRef(reject, listOptions);

Expand All @@ -288,15 +288,15 @@ export function listAll(listOptions: ListOptions): Promise<ListResult> {

const onFailureListener = new gmsTasks.OnFailureListener({
onFailure: exception => {
reject(exception.getCause() ? exception.getCause().getMessage() : exception.getMessage());
reject(exception);
}
});

const storageReference = storageRef.child(listOptions.remoteFullPath);

storageReference.listAll()
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);
.addOnSuccessListener(onSuccessListener)
.addOnFailureListener(onFailureListener);

} catch (ex) {
console.log("Error in firebase.listAll: " + ex);
Expand Down
22 changes: 11 additions & 11 deletions src/storage/storage.ios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function getReference(nativeReference: FIRStorageReference, listOptions: ListOpt
bucket: nativeReference.bucket,
name: nativeReference.name,
fullPath: nativeReference.fullPath,
listAll: (): Promise<ListResult> => listAll({remoteFullPath: nativeReference.fullPath, bucket: listOptions.bucket})
listAll: (): Promise<ListResult> => listAll({ remoteFullPath: nativeReference.fullPath, bucket: listOptions.bucket })
};
}

Expand Down Expand Up @@ -53,12 +53,12 @@ function getStorageRef(reject, arg): FIRStorageReference {
}
}

export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {
export function uploadFile(arg: UploadFileOptions): Promise<any> {
return new Promise((resolve, reject) => {
try {
const onCompletion = (metadata: FIRStorageMetadata, error: NSError) => {
if (error) {
reject(error.localizedDescription);
reject(error);
} else {
resolve({
name: metadata.name,
Expand Down Expand Up @@ -117,12 +117,12 @@ export function uploadFile(arg: UploadFileOptions): Promise<UploadFileResult> {
});
}

export function downloadFile(arg: DownloadFileOptions): Promise<string> {
export function downloadFile(arg: DownloadFileOptions): Promise<any> {
return new Promise((resolve, reject) => {
try {
const onCompletion = (url, error) => {
if (error) {
reject(error.localizedDescription);
reject(error);
} else {
resolve(url.absoluteString);
}
Expand Down Expand Up @@ -164,12 +164,12 @@ export function downloadFile(arg: DownloadFileOptions): Promise<string> {
});
}

export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<string> {
export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<any> {
return new Promise((resolve, reject) => {
try {
const onCompletion = (url, error) => {
if (error) {
reject(error.localizedDescription);
reject(error);
} else {
resolve(url.absoluteString);
}
Expand All @@ -191,12 +191,12 @@ export function getDownloadUrl(arg: GetDownloadUrlOptions): Promise<string> {
});
}

export function deleteFile(arg: DeleteFileOptions): Promise<void> {
return new Promise<void>((resolve, reject) => {
export function deleteFile(arg: DeleteFileOptions): Promise<any> {
return new Promise<any>((resolve, reject) => {
try {
const onCompletion = error => {
if (error) {
reject(error.localizedDescription);
reject(error);
} else {
resolve();
}
Expand Down Expand Up @@ -232,7 +232,7 @@ export function listAll(listOptions: ListOptions): Promise<ListResult> {

fIRStorageReference.listAllWithCompletion((result: FIRStorageListResult, error: NSError) => {
if (error) {
reject(error.localizedDescription);
reject(error);
} else {
resolve(new ListResult(result, listOptions));
}
Expand Down