feat: improve error handling

* remove "cleanup" parameter.
hendt · Mar 19, 2021 · c1fbdbb · c1fbdbb
1 parent 994887b
commit c1fbdbb
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 116 deletions.
diff --git a/README.md b/README.md
@@ -255,7 +255,6 @@ The second parameter in the traditional API has the following options:
 ```typescript
 export type Options = {
   raw?: boolean, // return raw XML
-  cleanup?: boolean, // remove extraneous tags like  '@', 'Ack', 
   parseOptions?: object, // https://github.com/NaturalIntelligence/fast-xml-parser
   useIaf?: boolean // use IAF in header instead of Bearer
   headers?: Headers // additional Headers (key, value)

diff --git a/src/api/restful/index.ts b/src/api/restful/index.ts
@@ -2,12 +2,13 @@ import debug from 'debug';
 import Auth from '../../auth/index';
 import {
   EBayAccessDenied,
+  EBayInvalidGrant,
   EBayInvalidScope,
   EBayNotFound,
   EBayUnauthorizedAfterRefresh,
-  getEBayError,
+  getEBayResponseError,
 } from '../../errors';
-import { createRequest, IEBayApiRequest } from '../../request';
+import {createRequest, IEBayApiRequest} from '../../request';
 
 const log = debug('ebay:restful:api');
 
@@ -142,7 +143,7 @@ export default abstract class Api {
     ex: any,
     refreshedToken?: boolean
   ): Promise<any> {
-    const error = getEBayError(ex);
+    const error = getEBayResponseError(ex);
 
     if (!error) {
       log('handleEBayError', ex);
@@ -151,13 +152,19 @@ export default abstract class Api {
 
     if (error.domain === 'ACCESS') {
       throw new EBayAccessDenied(ex);
-    } else if (error.message === 'Invalid access token') {
+    } else if (error.message === 'invalid_grant') {
+      throw new EBayInvalidGrant(ex);
+    } else if (error.errorId === EBayNotFound.code) {
+      throw new EBayNotFound(ex);
+    }
+
+    if (error.message === 'Invalid access token') {
       if (!refreshedToken) {
         // TODO extract this
         log('Token expired. Refresh the token.');
         return this.auth.oAuth2.refreshToken().catch((e: Error) => {
-          const responseError = getEBayError(e);
-          if (responseError && responseError.message === 'invalid_scope') {
+          const responseError = getEBayResponseError(e);
+          if (responseError?.message === 'invalid_scope') {
             throw new EBayInvalidScope(e);
           }
 
@@ -166,8 +173,6 @@ export default abstract class Api {
       }
 
       throw new EBayUnauthorizedAfterRefresh(ex);
-    } else if (error.errorId === 11001) {
-      throw new EBayNotFound(ex);
     }
 
     throw ex;

diff --git a/src/api/traditional/XMLRequest.ts b/src/api/traditional/XMLRequest.ts
@@ -5,15 +5,6 @@ import {EbayApiError, EBayIAFTokenExpired, EBayTokenRequired, NoCallError} from
 import {createRequest, IEBayApiRequest} from '../../request';
 import {Fields} from './fields';
 
-const EXTRANEOUS = [
-  '@',
-  'Ack',
-  'ack',
-  'Version',
-  'Build',
-  'xmlns'
-];
-
 const HEADING = '<?xml version="1.0" encoding="utf-8"?>';
 const log = debug('ebay:xml:request');
 
@@ -47,7 +38,6 @@ type Headers = {
 
 export type Options = {
   raw?: boolean,
-  cleanup?: boolean,
   parseOptions?: object,
   useIaf?: boolean,
   headers?: Headers
@@ -62,7 +52,6 @@ export type Config = {
 
 export const defaultOptions: Required<Options> = {
   raw: false,
-  cleanup: true,
   parseOptions: defaultXML2JSONParseOptions,
   useIaf: true,
   headers: {}
@@ -103,21 +92,10 @@ export default class XMLRequest {
     this.req = req;
   }
 
-  /**
-   * Delete extraneous fields from json.
-   *
-   * @param json
-   */
-  public static clean(json: any) {
-    EXTRANEOUS.forEach((key: string) => {
-      delete json[key]
-    })
-  }
-
   /**
    * converts an XML response to JSON
    *
-   * @param      {xml}     xml     The xml
+   * @param      {string}     xml     The xml
    * @param      {object}     parseOptions     The parse options
    * @return     {JSON}         resolves to a JSON representation of the HTML
    */
@@ -217,19 +195,21 @@ export default class XMLRequest {
 
       this.handleEBayJsonError(json);
 
-      // cleans the Ebay response
-      if (options.cleanup) {
-        XMLRequest.clean(json);
-      }
-
       return json;
     } catch (error) {
-      this.handleEBayResponseError(error, parseOptions);
+      log('eBayResponseError', error);
+
+      if (error.response?.data) {
+        const json = XMLRequest.toJSON(error.response.data, parseOptions);
+        this.handleEBayJsonError(json);
+      }
+
+      throw error;
     }
   }
 
   private handleEBayJsonError(json: any) {
-    if (json.Ack === 'Error' || json.Ack === 'Failure' || json.Errors) {
+    if (json.Errors?.ErrorCode) {
       switch (json.Errors.ErrorCode) {
         case EBayIAFTokenExpired.code:
           throw new EBayIAFTokenExpired(json);
@@ -238,17 +218,8 @@ export default class XMLRequest {
       }
 
       throw new EbayApiError(json.Errors);
-    }
-  }
-
-  private handleEBayResponseError(error: any, parseOptions: object) {
-    log('eBayResponseError', error);
-
-    if (error.response && error.response.data) {
-      const json = XMLRequest.toJSON(error.response.data, parseOptions);
-      this.handleEBayJsonError(json);
-    } else {
-      throw error;
+    } else if (json.errorMessage) {
+      throw new EbayApiError(json);
     }
   }
 }
diff --git a/src/api/traditional/index.ts b/src/api/traditional/index.ts
@@ -7,7 +7,7 @@ import {Fields} from './fields';
 import FindingCalls from './finding';
 import ShoppingCalls from './shopping';
 import TradingCalls from './trading';
-import {ClientAlerts, Finding, Shopping, Trading, TraditionalApi} from '../../types/traditonalTypes';
+import {ClientAlerts, Finding, Shopping, Trading, TraditionalApi} from '../../types';
 import XMLRequest, {defaultOptions, Options} from './XMLRequest';
 
 /**
@@ -110,7 +110,7 @@ export default class Traditional {
         };
 
         const service: any = {};
-        Object.keys(ClientAlertsCalls).forEach((callname: string) => {
+        Object.keys(api.calls).forEach((callname: string) => {
             service[callname] = async (fields: Fields) => {
                 return this.req.get(endpoint, {
                     paramsSerializer,
@@ -179,7 +179,7 @@ export default class Traditional {
 
     private createTraditionalXMLApi<T>(api: TraditionalApi): T {
         const service: any = {};
-        Object.keys(api.calls).map((callname: string) => {
+        Object.keys(api.calls).forEach((callname: string) => {
             service[callname] = this.createXMLRequest(callname, api);
         });
 

diff --git a/src/errors/index.ts b/src/errors/index.ts
@@ -5,7 +5,6 @@
  * @ignore
  */
 abstract class EBayError extends Error {
-
     public meta: any = null;
 
     /**
@@ -23,23 +22,12 @@ abstract class EBayError extends Error {
     }
 }
 
-/**
- * thrown when Request.prototype.run() is called without an oAuth2
- *
- * @class      No_Auth_Token (name)
- */
-export class NoAuthTokenError extends EBayError {
-    constructor(msg = 'no oAuth2 present.  Please invoke `Ebay.prototype.oAuth2(<Token>)`.') {
-        super(msg);
-    }
-}
-
 /**
  * thrown when Request.prototype.run() is called without having defined an eBay API call
  */
 
 export class NoCallError extends EBayError {
-    constructor(msg = 'no eBay API call defined, please invoke one.') {
+    constructor(msg = 'No eBay API call defined, please invoke one.') {
         super(msg);
     }
 }
@@ -49,89 +37,86 @@ export class NoCallError extends EBayError {
  */
 export class EnvError extends EBayError {
     constructor(key: any) {
-        super(`could not find ${key} in process.env`);
+        super(`Could not find ${key} in process.env.`);
     }
 }
 
 /**
  * Thrown when an Error occurs on eBay's side.
  */
 export class EbayApiError extends EBayError {
-    constructor(err: any, name = 'EbayApiError') {
+    public readonly name: string;
+
+    constructor(err: any, name?: string) {
         let message = '';
-        const resError = getEBayError(err);
+        const resError = getEBayResponseError(err);
         if (resError) {
-            message = resError.message;
+            message = resError.message + ': ' + resError.error_description;
         } else if (err.errorMessage) {
             message = err.errorMessage.error.message;
+        } else if (err.Errors) {
+            message = err.Errors.LongMessage || err.Errors.ShortMessage;
         } else {
             message = err.LongMessage || err.ShortMessage;
         }
 
         super(message);
         this.meta = err;
-        this.name = name;
+        this.name = name || this.constructor.name;
     }
 }
 
 export class EBayAccessDenied extends EBayError {
     constructor(err: any) {
         super('Access denied');
+        this.meta = err.response?.data;
+    }
+}
+
+export class EBayInvalidGrant extends EBayError {
+    constructor(err: any) {
+        super(err.response.data.error_description);
         this.meta = err.response.data;
-        this.name = 'EBayAccessDenied';
+        this.name = 'EBayInvalidGrant';
     }
 }
 
 export class EBayNotFound extends EBayError {
+    public static readonly code = 11001;
+
     constructor(err: any) {
         super(err.message);
         this.meta = err.response.data;
-        this.name = 'EBayEBayNotFound';
     }
 }
 
 export class EBayUnauthorizedAfterRefresh extends EBayError {
     constructor(err: any) {
         super('Unauthorized after refreshing token.');
         this.meta = err.response.data;
-        this.name = 'EBayUnauthorized';
     }
 }
 
 export class EBayIAFTokenExpired extends EbayApiError {
-    public static code = 21917053;
-
-    constructor(err: any) {
-        super(err, 'EBayIAFTokenExpired');
-    }
+    public static readonly code = 21917053;
 }
 
 export class EBayTokenRequired extends EbayApiError {
-    public static code = 930;
-
-    constructor(err: any) {
-        super(err, 'EBayTokenRequired');
-    }
+    public static readonly code = 930;
 }
 
-export class EBayInvalidScope extends EBayError {
-    constructor(err: any) {
-        super(err.response.data.error_description);
-        this.meta = err.response.data;
-        this.name = 'EBayInvalidScope';
-    }
-}
+export class EBayInvalidScope extends EbayApiError {}
 
-export const getEBayError = (e: any) => {
-    if (e.response && e.response.data) {
-        const data = e.response.data;
-        if (data.error) {
-            return {
-                message: data.error,
-                description: data.error_description
-            };
+export const getEBayResponseError = (e: any) => {
+    if (e.response?.data?.error) {
+        return {
+            message: e.response.data.error,
+            description: e.response.data.error_description
         }
-        return data.errors && data.errors[0] ? data.errors[0] : null;
+    }
+
+    if (e.response?.data?.errors) {
+        return e.response?.data?.errors[0] ?? null;
     }
 
     return null;

diff --git a/test/api/traditional/xmlRequest.spec.ts b/test/api/traditional/xmlRequest.spec.ts
@@ -60,22 +60,6 @@ describe('XMLRequestTest', () => {
     });
   });
 
-  it('Removes Extraneous nodes', () => {
-    const response = `<?xml version="1.0" encoding="utf-8"?>
-<CALLResponse xmlns="urn:ebay:apis:eBLBaseComponents">
-  <Ack>Ack</Timestamp>
-  <ack>ack</Ack>
-  <Version>Version</Version>
-  <Build>Build</Build>
-</CALLResponse>`;
-
-    req.post = sinon.stub().returns(Promise.resolve(response));
-    const request = new XMLRequest('CALL', {}, config, req);
-    return request.fetch().then(result => {
-      expect(result).to.deep.equal({});
-    });
-  });
-
   it('Unwraps Response', () => {
     const response = `<?xml version="1.0" encoding="utf-8"?>
 <CALLResponse xmlns="urn:ebay:apis:eBLBaseComponents">