feat: Add support for string custom error messages

With this release you no longer have to return error to describe the custom error message. Instead, you can simply return a string from the rule - `shield` will handle the rest.
maticzav · Dec 22, 2018 · 992e8b3 · 992e8b3
1 parent d851554
commit 992e8b3
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 49 deletions.
diff --git a/README.md b/README.md
@@ -167,11 +167,11 @@ function rule(
   options?: IRuleOptions,
 ): (func: IRuleFunction) => Rule
 
-export type IFragment = string
-export type ICacheOptions = 'strict' | 'contextual' | 'no_cache' | boolean
-export type IRuleResult = boolean | Error
+type IFragment = string
+type ICacheOptions = 'strict' | 'contextual' | 'no_cache' | boolean
+type IRuleResult = boolean | string | Error
 
-export type IRuleFunction = (
+type IRuleFunction = (
   parent?: any,
   args?: any,
   context?: any,
@@ -190,7 +190,7 @@ function not(rule: IRule): LogicRule
 const allow: LogicRule
 const deny: LogicRule
 
-export type ShieldRule = IRule | ILogicRule
+type ShieldRule = IRule | ILogicRule
 
 interface IRuleFieldMap {
   [key: string]: IRule
@@ -200,11 +200,11 @@ interface IRuleTypeMap {
   [key: string]: IRule | IRuleFieldMap
 }
 
-export type IRules = ShieldRule | IRuleTypeMap
+type IRules = ShieldRule | IRuleTypeMap
 
 function shield(rules?: IRules, options?: IOptions): IMiddleware
 
-export interface IOptions {
+interface IOptions {
   debug?: boolean
   allowExternalErrors?: boolean
   fallbackRule?: ShieldRule
@@ -266,7 +266,7 @@ const admin = rule({ cache: 'strict' })(async (parent, args, ctx, info) => {
 
 Shield, by default, catches all errors thrown during resolver execution. This way we can be 100% sure none of your internal logic can be exposed to the client if it was not meant to be.
 
-To return custom error messages to your client, you can return error instead of throwing it. This way, Shield knows it's not a bug but rather a design decision under control.
+To return custom error messages to your client, you can return error instead of throwing it. This way, Shield knows it's not a bug but rather a design decision under control. Besides returning an error you can also return a `string` representing a custom error message.
 
 You can return custom error from resolver or from rule itself. Rules that return error are treated as failing, therefore not processing any further resolvers.
 
@@ -283,6 +283,11 @@ const resolvers = {
     customErrorInResolver: () => {
       return new Error('Custom error message from resolver.')
     },
+    customErrorMessageInRule: () => {
+      // Querying is stopped because rule returns an error
+      console.log("This won't be logged.")
+      return "you won't see me!"
+    },
     customErrorInRule: () => {
       // Querying is stopped because rule returns an error
       console.log("This won't be logged.")
@@ -292,12 +297,17 @@ const resolvers = {
 }
 
 const ruleWithCustomError = rule()(async (parent, args, ctx, info) => {
-  return new Error('Custom error message from rule.')
+  return new Error('Custom error from rule.')
+})
+
+const ruleWithCustomErrorMessage = rule()(async (parent, args, ctx, info) => {
+  return 'Custom error message from rule.'
 })
 
 const permissions = shield({
   Query: {
     customErrorInRule: ruleWithCustomError,
+    customErrorMessageInRule: ruleWithCustomErrorMessage,
   },
 })
 

diff --git a/src/rules.ts b/src/rules.ts
@@ -60,6 +60,8 @@ export class Rule implements IRule {
 
       if (res instanceof Error) {
         return res
+      } else if (typeof res === 'string') {
+        return new Error(res)
       } else if (res === true) {
         return true
       } else {

diff --git a/src/test/fallback.test.ts b/src/test/fallback.test.ts
@@ -100,7 +100,7 @@ describe('fallbackError correctly handles errors', () => {
     expect(res.errors[0].message).toBe(fallbackError.message)
   })
 
-  test('error in resolver with allowExternalErrors returns external error.', async () => {
+  test('correctly converts string fallbackError to error fallbackError', async () => {
     /* Schema */
 
     const typeDefs = `
@@ -112,7 +112,7 @@ describe('fallbackError correctly handles errors', () => {
     const resolvers = {
       Query: {
         test: () => {
-          throw new Error('external')
+          throw new Error()
         },
       },
     }
@@ -121,19 +121,19 @@ describe('fallbackError correctly handles errors', () => {
 
     /* Permissions */
 
+    const fallbackMessage = Math.random().toString()
     const permissions = shield(
       {
         Query: allow,
       },
       {
-        allowExternalErrors: true,
+        fallbackError: fallbackMessage,
       },
     )
 
     const schemaWithPermissions = applyMiddleware(schema, permissions)
 
     /* Execution */
-
     const query = `
       query {
         test
@@ -144,24 +144,31 @@ describe('fallbackError correctly handles errors', () => {
     /* Tests */
 
     expect(res.data).toBeNull()
-    expect(res.errors[0].message).toBe('external')
+    expect(res.errors[0].message).toBe(fallbackMessage)
   })
+})
 
-  test('error in rule with allowExternalErrors returns fallback.', async () => {
+describe('external errors can be controled correctly', () => {
+  test('error in resolver with allowExternalErrors returns external error.', async () => {
     /* Schema */
+
     const typeDefs = `
       type Query {
         test: String!
       }
     `
 
-    const schema = makeExecutableSchema({ typeDefs, resolvers: {} })
+    const resolvers = {
+      Query: {
+        test: () => {
+          throw new Error('external')
+        },
+      },
+    }
 
-    /* Permissions */
+    const schema = makeExecutableSchema({ typeDefs, resolvers })
 
-    const allow = rule()(() => {
-      throw new Error('external')
-    })
+    /* Permissions */
 
     const permissions = shield(
       {
@@ -186,12 +193,11 @@ describe('fallbackError correctly handles errors', () => {
     /* Tests */
 
     expect(res.data).toBeNull()
-    expect(res.errors[0].message).toBe('Not Authorised!')
+    expect(res.errors[0].message).toBe('external')
   })
 
-  test('custom error in rule returns custom error.', async () => {
+  test('error in rule with allowExternalErrors returns fallback.', async () => {
     /* Schema */
-
     const typeDefs = `
       type Query {
         test: String!
@@ -202,16 +208,23 @@ describe('fallbackError correctly handles errors', () => {
 
     /* Permissions */
 
-    const error = new Error(`${Math.random()}`)
-    const permissions = shield({
-      Query: rule()(() => {
-        return error
-      }),
+    const allow = rule()(() => {
+      throw new Error('external')
     })
 
+    const permissions = shield(
+      {
+        Query: allow,
+      },
+      {
+        allowExternalErrors: true,
+      },
+    )
+
     const schemaWithPermissions = applyMiddleware(schema, permissions)
 
     /* Execution */
+
     const query = `
       query {
         test
@@ -222,9 +235,11 @@ describe('fallbackError correctly handles errors', () => {
     /* Tests */
 
     expect(res.data).toBeNull()
-    expect(res.errors[0].message).toBe(error.message)
+    expect(res.errors[0].message).toBe('Not Authorised!')
   })
+})
 
+describe('debug mode works as expected', () => {
   test('returns original error in debug mode when rule error occurs', async () => {
     /* Schema */
     const typeDefs = `
@@ -313,8 +328,10 @@ describe('fallbackError correctly handles errors', () => {
     expect(res.data).toBeNull()
     expect(res.errors[0].message).toBe('debug')
   })
+})
 
-  test('correctly converts string fallbackError to error fallbackError', async () => {
+describe('custom errors work as expected', () => {
+  test('custom error in rule returns custom error.', async () => {
     /* Schema */
 
     const typeDefs = `
@@ -323,27 +340,53 @@ describe('fallbackError correctly handles errors', () => {
       }
     `
 
-    const resolvers = {
-      Query: {
-        test: () => {
-          throw new Error()
-        },
-      },
-    }
+    const schema = makeExecutableSchema({ typeDefs, resolvers: {} })
 
-    const schema = makeExecutableSchema({ typeDefs, resolvers })
+    /* Permissions */
+
+    const error = new Error(`${Math.random()}`)
+    const permissions = shield({
+      Query: rule()(() => {
+        return error
+      }),
+    })
+
+    const schemaWithPermissions = applyMiddleware(schema, permissions)
+
+    /* Execution */
+    const query = `
+      query {
+        test
+      }
+    `
+    const res = await graphql(schemaWithPermissions, query)
+
+    /* Tests */
+
+    expect(res.data).toBeNull()
+    expect(res.errors[0].message).toBe(error.message)
+  })
+
+  test('custom error message in rule returns custom error.', async () => {
+    /* Schema */
+
+    const typeDefs = `
+      type Query {
+        test: String!
+      }
+    `
+
+    const schema = makeExecutableSchema({ typeDefs, resolvers: {} })
 
     /* Permissions */
 
-    const fallbackMessage = Math.random().toString()
-    const permissions = shield(
-      {
-        Query: allow,
-      },
-      {
-        fallbackError: fallbackMessage,
-      },
-    )
+    const error = `${Math.random()}`
+
+    const permissions = shield({
+      Query: rule()(() => {
+        return error
+      }),
+    })
 
     const schemaWithPermissions = applyMiddleware(schema, permissions)
 
@@ -358,7 +401,7 @@ describe('fallbackError correctly handles errors', () => {
     /* Tests */
 
     expect(res.data).toBeNull()
-    expect(res.errors[0].message).toBe(fallbackMessage)
+    expect(res.errors[0].message).toBe(error)
   })
 })
 

diff --git a/src/types.ts b/src/types.ts
@@ -31,7 +31,7 @@ export declare class ILogicRule {
 
 export type IFragment = string
 export type ICache = 'strict' | 'contextual' | 'no_cache'
-export type IRuleResult = boolean | Error
+export type IRuleResult = boolean | string | Error
 export type IRuleFunction = (
   parent?: any,
   args?: any,