Merge pull request #2 from StatesTitle/mdoliner-st/Change_exceptions_…

…to_extend_Error

I'm merging this change into the all_states_title_changes branch. For the full PR description see [the PR for merging this change into Credit Karma's master branch](creditkarma#184).

README.md

@@ -320,24 +320,26 @@ Given Thrift:
 
 ```c
 exception MyException {
-    1: string message
+    1: string msg
     2: i32 code
 }
 ```
 
 Generated TypeScript:
 
 ```typescript
-export class MyException extends thrift.StructLike implements IMyException {
-    public message: string
+export class MyException extends thrift.ErrorStructLike implements IMyException {
+    public msg: string
     public code?: number
-    constructor(args?: { message?: string, code?: number }) {
+    constructor(args?: { msg?: string, code?: number }) {
         // ...
     }
 }
 ```
 
-Then in your service client you could just throw the exception as you would any JS error `throw new MyException({ message: 'whoops', code: 500 });`
+Then in your service client you could just throw the exception as you would any JS error `throw new MyException({ msg: 'whoops', code: 500 });`
+
+Note that these exception classes extend thrift.ErrorStructLike which extends JavaScript's Error object. Because of this you should avoid having fields in your exceptions that shadow fields in Error. `thrift-typescript` doesn't warn about this (TODO: Though perhaps it should try?) See [the MDN JavaScript reference](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Instance_properties) for a list of properties to avoid. (You actually *can* use the same field names if you want, as long as the types align, but avoiding them entirely might result in fewer headaches.)
 
 #### Service
 

src/main/render/thrift-server/exception/index.ts

@@ -10,5 +10,5 @@ export function renderException(
     node: ExceptionDefinition,
     state: IRenderState,
 ): Array<ts.Statement> {
-    return renderStruct(node, state)
+    return renderStruct(node, state, true)
 }

src/main/render/thrift-server/identifiers.ts

@@ -33,6 +33,7 @@ export const THRIFT_IDENTIFIERS = {
         'thrift.InputBufferUnderrunError',
     ),
     StructLike: ts.createIdentifier('thrift.StructLike'),
+    ErrorStructLike: ts.createIdentifier('thrift.ErrorStructLike'),
 }
 
 export const THRIFT_TYPES = {

src/main/render/thrift-server/struct/class.ts

@@ -27,6 +27,7 @@ import {
     classNameForStruct,
     createSuperCall,
     extendsAbstract,
+    extendsAbstractError,
     implementsInterface,
     looseNameForStruct,
     throwForField,
@@ -38,6 +39,7 @@ export function renderClass(
     node: InterfaceWithFields,
     state: IRenderState,
     isExported: boolean,
+    extendError: boolean = false,
 ): ts.ClassDeclaration {
     const fields: Array<ts.PropertyDeclaration> = [
         ...createFieldsForStruct(node, state),
@@ -97,7 +99,10 @@ export function renderClass(
         tokens(isExported),
         classNameForStruct(node, state).replace('__NAMESPACE__', ''),
         [],
-        [extendsAbstract(), implementsInterface(node, state)], // heritage
+        [
+            extendError ? extendsAbstractError() : extendsAbstract(),
+            implementsInterface(node, state),
+        ], // heritage
         [
             ...fields,
             ctor,

src/main/render/thrift-server/struct/index.ts

@@ -13,10 +13,11 @@ import { renderClass } from './class'
 export function renderStruct(
     node: InterfaceWithFields,
     state: IRenderState,
+    extendError: boolean = false,
 ): Array<ts.Statement> {
     return [
         ...renderInterface(node, state, true),
         renderToolkit(node, state, true),
-        renderClass(node, state, true),
+        renderClass(node, state, true, extendError),
     ]
 }

src/main/render/thrift-server/struct/utils.ts

@@ -133,6 +133,15 @@ export function extendsAbstract(): ts.HeritageClause {
     ])
 }
 
+export function extendsAbstractError(): ts.HeritageClause {
+    return ts.createHeritageClause(ts.SyntaxKind.ExtendsKeyword, [
+        ts.createExpressionWithTypeArguments(
+            [],
+            THRIFT_IDENTIFIERS.ErrorStructLike,
+        ),
+    ])
+}
+
 export function implementsInterface(
     node: InterfaceWithFields,
     state: IRenderState,

src/tests/unit/fixtures/thrift-server/annotations_exception.solution.ts

@@ -71,7 +71,7 @@ export const MyExceptionCodec: thrift.IStructCodec<IMyExceptionArgs, IMyExceptio
         };
     }
 };
-export class MyException extends thrift.StructLike implements IMyException {
+export class MyException extends thrift.ErrorStructLike implements IMyException {
     public message?: string;
     public code?: number = 200;
     public readonly __name = "MyException";

src/tests/unit/fixtures/thrift-server/basic_exception.solution.ts

@@ -71,7 +71,7 @@ export const MyExceptionCodec: thrift.IStructCodec<IMyExceptionArgs, IMyExceptio
         };
     }
 };
-export class MyException extends thrift.StructLike implements IMyException {
+export class MyException extends thrift.ErrorStructLike implements IMyException {
     public message?: string;
     public code?: number = 200;
     public readonly __name = "MyException";

src/tests/unit/fixtures/thrift-server/generated-strict/com/test/calculator/NotAGoodIdea.ts

@@ -93,7 +93,7 @@ export const NotAGoodIdeaCodec: thrift.IStructCodec<INotAGoodIdeaArgs, INotAGood
         };
     }
 };
-export class NotAGoodIdea extends thrift.StructLike implements INotAGoodIdea {
+export class NotAGoodIdea extends thrift.ErrorStructLike implements INotAGoodIdea {
     public message?: string;
     public data?: TypedMap.TypedMap;
     public readonly __name = "NotAGoodIdea";

src/tests/unit/fixtures/thrift-server/generated-strict/com/test/common/AuthException.ts

@@ -78,7 +78,7 @@ export const AuthExceptionCodec: thrift.IStructCodec<IAuthExceptionArgs, IAuthEx
         };
     }
 };
-export class AuthException extends thrift.StructLike implements IAuthException {
+export class AuthException extends thrift.ErrorStructLike implements IAuthException {
     public code?: number;
     public message?: string;
     public readonly __name = "AuthException";

src/tests/unit/fixtures/thrift-server/generated-strict/com/test/exceptions/InvalidOperation.ts

@@ -78,7 +78,7 @@ export const InvalidOperationCodec: thrift.IStructCodec<IInvalidOperationArgs, I
         };
     }
 };
-export class InvalidOperation extends thrift.StructLike implements IInvalidOperation {
+export class InvalidOperation extends thrift.ErrorStructLike implements IInvalidOperation {
     public whatOp?: number;
     public why?: string;
     public readonly __name = "InvalidOperation";

src/tests/unit/fixtures/thrift-server/generated-strict/com/test/exceptions/InvalidResult.ts

@@ -79,7 +79,7 @@ export const InvalidResultCodec: thrift.IStructCodec<IInvalidResultArgs, IInvali
         };
     }
 };
-export class InvalidResult extends thrift.StructLike implements IInvalidResult {
+export class InvalidResult extends thrift.ErrorStructLike implements IInvalidResult {
     public message?: string;
     public code?: __ROOT_NAMESPACE__.ICode;
     public readonly __name = "InvalidResult";

src/tests/unit/fixtures/thrift-server/generated/com/test/calculator/NotAGoodIdea.ts

@@ -93,7 +93,7 @@ export const NotAGoodIdeaCodec: thrift.IStructCodec<INotAGoodIdeaArgs, INotAGood
         };
     }
 };
-export class NotAGoodIdea extends thrift.StructLike implements INotAGoodIdea {
+export class NotAGoodIdea extends thrift.ErrorStructLike implements INotAGoodIdea {
     public message?: string;
     public data?: TypedMap.TypedMap;
     public readonly __name = "NotAGoodIdea";

src/tests/unit/fixtures/thrift-server/generated/com/test/common/AuthException.ts

@@ -78,7 +78,7 @@ export const AuthExceptionCodec: thrift.IStructCodec<IAuthExceptionArgs, IAuthEx
         };
     }
 };
-export class AuthException extends thrift.StructLike implements IAuthException {
+export class AuthException extends thrift.ErrorStructLike implements IAuthException {
     public code?: number;
     public message?: string;
     public readonly __name = "AuthException";

src/tests/unit/fixtures/thrift-server/generated/com/test/exceptions/InvalidOperation.ts

@@ -78,7 +78,7 @@ export const InvalidOperationCodec: thrift.IStructCodec<IInvalidOperationArgs, I
         };
     }
 };
-export class InvalidOperation extends thrift.StructLike implements IInvalidOperation {
+export class InvalidOperation extends thrift.ErrorStructLike implements IInvalidOperation {
     public whatOp?: number;
     public why?: string;
     public readonly __name = "InvalidOperation";

src/tests/unit/fixtures/thrift-server/generated/com/test/exceptions/InvalidResult.ts

@@ -79,7 +79,7 @@ export const InvalidResultCodec: thrift.IStructCodec<IInvalidResultArgs, IInvali
         };
     }
 };
-export class InvalidResult extends thrift.StructLike implements IInvalidResult {
+export class InvalidResult extends thrift.ErrorStructLike implements IInvalidResult {
     public message?: string;
     public code?: __ROOT_NAMESPACE__.ICode;
     public readonly __name = "InvalidResult";

src/tests/unit/fixtures/thrift-server/nested_exception.solution.ts

@@ -179,7 +179,7 @@ export const MyExceptionCodec: thrift.IStructCodec<IMyExceptionArgs, IMyExceptio
         }
     }
 };
-export class MyException extends thrift.StructLike implements IMyException {
+export class MyException extends thrift.ErrorStructLike implements IMyException {
     public description: string;
     public code?: ICode;
     public readonly __name = "MyException";

src/tests/unit/fixtures/thrift-server/required_field_exception.solution.ts

@@ -79,7 +79,7 @@ export const MyExceptionCodec: thrift.IStructCodec<IMyExceptionArgs, IMyExceptio
         }
     }
 };
-export class MyException extends thrift.StructLike implements IMyException {
+export class MyException extends thrift.ErrorStructLike implements IMyException {
     public description: string;
     public code?: number;
     public readonly __name = "MyException";

src/tests/unit/fixtures/thrift-server/throws_multi_service.solution.ts

@@ -53,7 +53,7 @@ export const ServiceExceptionCodec: thrift.IStructCodec<IServiceExceptionArgs, I
         };
     }
 };
-export class ServiceException extends thrift.StructLike implements IServiceException {
+export class ServiceException extends thrift.ErrorStructLike implements IServiceException {
     public message?: string;
     public readonly __name = "ServiceException";
     public readonly _annotations: thrift.IThriftAnnotations = {};
@@ -148,7 +148,7 @@ export const AuthExceptionCodec: thrift.IStructCodec<IAuthExceptionArgs, IAuthEx
         };
     }
 };
-export class AuthException extends thrift.StructLike implements IAuthException {
+export class AuthException extends thrift.ErrorStructLike implements IAuthException {
     public message?: string;
     public code?: number;
     public readonly __name = "AuthException";
@@ -230,7 +230,7 @@ export const UnknownExceptionCodec: thrift.IStructCodec<IUnknownExceptionArgs, I
         };
     }
 };
-export class UnknownException extends thrift.StructLike implements IUnknownException {
+export class UnknownException extends thrift.ErrorStructLike implements IUnknownException {
     public message?: string;
     public readonly __name = "UnknownException";
     public readonly _annotations: thrift.IThriftAnnotations = {};

src/tests/unit/fixtures/thrift-server/throws_service.solution.ts

@@ -53,7 +53,7 @@ export const ServiceExceptionCodec: thrift.IStructCodec<IServiceExceptionArgs, I
         };
     }
 };
-export class ServiceException extends thrift.StructLike implements IServiceException {
+export class ServiceException extends thrift.ErrorStructLike implements IServiceException {
     public message?: string;
     public readonly __name = "ServiceException";
     public readonly _annotations: thrift.IThriftAnnotations = {};