Better error handling (#144)

Check fetch responses for non-success responses, and throw errors accordingly.
stellar · Mar 6, 2020 · 98acc15 · 98acc15
1 parent ee9c15b
commit 98acc15
Show file tree

Hide file tree

Showing 5 changed files with 134 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## In master
 
+- [KeyManager] Correctly handle non-successful HTTP status codes when fetching
+  auth tokens.
 - [Transfers] Make the transactions fetcher more resilient to invalid responses
   from /transactions.
 

diff --git a/src/KeyManager.ts b/src/KeyManager.ts
@@ -307,6 +307,22 @@ export class KeyManager {
       `${authServer}?account=${encodeURIComponent(key.publicKey)}`,
     );
 
+    if (challengeRes.status !== 200) {
+      try {
+        const challengeJson = await challengeRes.json();
+        throw new Error(
+          `[KeyManager#fetchAuthToken] Failed to fetch a challenge transaction, 
+          error: ${challengeJson.error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `[KeyManager#fetchAuthToken] Failed to fetch a challenge transaction, 
+          error code ${challengeRes.status} and status text 
+          "${challengeRes.statusText}"`,
+        );
+      }
+    }
+
     const keyNetwork = key.network || this.defaultNetworkPassphrase;
 
     const {
@@ -353,6 +369,22 @@ export class KeyManager {
       },
     });
 
+    if (responseRes.status !== 200) {
+      try {
+        const responseJson = await responseRes.json();
+        throw new Error(
+          `[KeyManager#fetchAuthToken] Failed to return a signed transaction, 
+          error: ${responseJson.error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `[KeyManager#fetchAuthToken] Failed to return a signed transaction, 
+          error code ${responseRes.status} and status text 
+          "${responseRes.statusText}"`,
+        );
+      }
+    }
+
     const { token, message, status } = await responseRes.json();
 
     // if we get a false status message, error out

diff --git a/src/transfers/DepositProvider.ts b/src/transfers/DepositProvider.ts
@@ -110,7 +110,7 @@ export class DepositProvider extends TransferProvider {
     };
     const isAuthRequired = this.getAuthStatus("deposit", params.asset_code);
 
-    let text;
+    let response;
     let json;
 
     if (shouldUseNewEndpoints) {
@@ -120,24 +120,37 @@ export class DepositProvider extends TransferProvider {
         body.append(key, request[key]);
       });
 
-      const response = await fetch(
+      response = await fetch(
         `${this.transferServer}/transactions/deposit/interactive`,
         {
           method: "POST",
           body,
           headers: isAuthRequired ? this.getHeaders() : undefined,
         },
       );
-
-      text = await response.text();
     } else {
       const qs = queryString.stringify(request);
-      const response = await fetch(`${this.transferServer}/deposit?${qs}`, {
+      response = await fetch(`${this.transferServer}/deposit?${qs}`, {
         headers: isAuthRequired ? this.getHeaders() : undefined,
       });
-      text = await response.text();
     }
 
+    if (!response.ok) {
+      try {
+        const { error } = await response.json();
+        throw new Error(
+          `Error starting deposit to ${this.transferServer}: error ${error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `Error starting deposit to ${this.transferServer}: error 
+          code ${response.status}, status text: "${response.statusText}"`,
+        );
+      }
+    }
+
+    const text = await response.text();
+
     try {
       json = JSON.parse(text) as TransferResponse;
     } catch (e) {

diff --git a/src/transfers/TransferProvider.ts b/src/transfers/TransferProvider.ts
@@ -112,6 +112,21 @@ export abstract class TransferProvider {
 
   protected async fetchInfo(): Promise<Info> {
     const response = await fetch(`${this.transferServer}/info`);
+
+    if (!response.ok) {
+      try {
+        const { error } = await response.json();
+        throw new Error(
+          `Error fetching info from ${this.transferServer}: error ${error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `Error fetching info from ${this.transferServer}: error 
+          code ${response.status}, status text: "${response.statusText}"`,
+        );
+      }
+    }
+
     const rawInfo = (await response.json()) as RawInfoResponse;
     const info = parseInfo(rawInfo);
     this.info = info;
@@ -175,6 +190,22 @@ export abstract class TransferProvider {
       },
     );
 
+    if (!response.ok) {
+      try {
+        const { error } = await response.json();
+        throw new Error(
+          `Error fetching transactions from ${
+            this.transferServer
+          }: error ${error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `Error fetching transactions from ${this.transferServer}: error 
+          code ${response.status}, status text: "${response.statusText}"`,
+        );
+      }
+    }
+
     const text = await response.text();
 
     try {
@@ -235,6 +266,22 @@ export abstract class TransferProvider {
       },
     );
 
+    if (!response.ok) {
+      try {
+        const { error } = await response.json();
+        throw new Error(
+          `Error fetching transaction ${qs} from ${
+            this.transferServer
+          }: error ${error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `Error fetching transaction ${qs} from ${this.transferServer}: error 
+          code ${response.status}, status text: "${response.statusText}"`,
+        );
+      }
+    }
+
     const { transaction }: { transaction: Transaction } = await response.json();
 
     return _normalizeTransaction(transaction);
@@ -569,6 +616,21 @@ export abstract class TransferProvider {
             operation: this.operation,
           })}`,
         );
+
+        if (!response.ok) {
+          try {
+            const { error } = await response.json();
+            throw new Error(
+              `Error fetching fees from ${this.transferServer}: error ${error}`,
+            );
+          } catch (e) {
+            throw new Error(
+              `Error fetching fees from ${this.transferServer}: error 
+              code ${response.status}, status text: "${response.statusText}"`,
+            );
+          }
+        }
+
         const { fee: feeResponse } = await response.json();
         return feeResponse as number;
 

diff --git a/src/transfers/WithdrawProvider.ts b/src/transfers/WithdrawProvider.ts
@@ -103,7 +103,7 @@ export class WithdrawProvider extends TransferProvider {
     };
     const isAuthRequired = this.getAuthStatus("withdraw", params.asset_code);
 
-    let text;
+    let response;
     let json;
 
     if (shouldUseNewEndpoints) {
@@ -113,24 +113,37 @@ export class WithdrawProvider extends TransferProvider {
         body.append(key, request[key]);
       });
 
-      const response = await fetch(
+      response = await fetch(
         `${this.transferServer}/transactions/withdraw/interactive`,
         {
           method: "POST",
           body,
           headers: isAuthRequired ? this.getHeaders() : undefined,
         },
       );
-
-      text = await response.text();
     } else {
       const qs = queryString.stringify(request);
-      const response = await fetch(`${this.transferServer}/withdraw?${qs}`, {
+      response = await fetch(`${this.transferServer}/withdraw?${qs}`, {
         headers: isAuthRequired ? this.getHeaders() : undefined,
       });
-      text = await response.text();
     }
 
+    if (!response.ok) {
+      try {
+        const { error } = await response.json();
+        throw new Error(
+          `Error starting withdrawal to ${this.transferServer}: error ${error}`,
+        );
+      } catch (e) {
+        throw new Error(
+          `Error starting withdrawal to ${this.transferServer}: error 
+          code ${response.status}, status text: "${response.statusText}"`,
+        );
+      }
+    }
+
+    const text = await response.text();
+
     try {
       json = JSON.parse(text) as TransferResponse;
     } catch (e) {