Repository: artusjs/core
Pull request #211 (2022-10-20 11:18:32)

fix(exception): exception-filter loader

codecov-commenter: # [Codecov](https://codecov.io/gh/artusjs/core/pull/211?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
Base: **92.26**% // Head: **92.22**% // Decreases project coverage by **`-0.03%`** :warning:
> Coverage data is based on head [(`db5a8bb`)](https://codecov.io/gh/artusjs/core/pull/211?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) compared to base [(`2931407`)](https://codecov.io/gh/artusjs/core/commit/29314073705beb60d1240c68a4f50b25dad466a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> Patch coverage: 100.00% of modified lines in pull request are covered.

> :exclamation: Current head db5a8bb differs from pull request most recent head 529e605. Consider uploading reports for the commit 529e605 to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
- Coverage   92.26%   92.22%   -0.04%     
==========================================
  Files          55       55              
  Lines        1292     1287       -5     
  Branches      210      202       -8     
==========================================
- Hits         1192     1187       -5     
  Misses        100      100              
```


| [Impacted Files](https://codecov.io/gh/artusjs/core/pull/211?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/constant.ts](https://codecov.io/gh/artusjs/core/pull/211/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbnN0YW50LnRz) | `100.00% <ø> (ø)` | |
| [src/exception/decorator.ts](https://codecov.io/gh/artusjs/core/pull/211/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2V4Y2VwdGlvbi9kZWNvcmF0b3IudHM=) | `100.00% <100.00%> (ø)` | |
| [src/exception/utils.ts](https://codecov.io/gh/artusjs/core/pull/211/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2V4Y2VwdGlvbi91dGlscy50cw==) | `100.00% <100.00%> (ø)` | |
| [src/loader/impl/exception\_filter.ts](https://codecov.io/gh/artusjs/core/pull/211/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2xvYWRlci9pbXBsL2V4Y2VwdGlvbl9maWx0ZXIudHM=) | `90.90% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/artusjs/core/pull/211?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/src/constant.ts b/src/constant.ts
index 7a081e6..b4bf105 100644
--- a/src/constant.ts
+++ b/src/constant.ts
@@ -52,6 +52,7 @@ export const EXCEPTION_FILENAME = 'exception.json';
 
 export const DEFAULT_LOADER_LIST_WITH_ORDER = [
   'exception',
+  'exception-filter',
   'plugin-config',
   'plugin-meta',
   'framework-config',
diff --git a/src/exception/decorator.ts b/src/exception/decorator.ts
index 5a00b01..3fc574f 100644
--- a/src/exception/decorator.ts
+++ b/src/exception/decorator.ts
@@ -7,7 +7,7 @@ export const Catch = (targetErr?: string|Constructable<Error>): ClassDecorator =
     Reflect.defineMetadata(EXCEPTION_FILTER_METADATA_KEY, {
       targetErr: targetErr ?? EXCEPTION_FILTER_DEFAULT_SYMBOL,
     }, target);
-    Reflect.defineMetadata(HOOK_FILE_LOADER, { loader: 'exception_filter' }, target);
+    Reflect.defineMetadata(HOOK_FILE_LOADER, { loader: 'exception-filter' }, target);
     Injectable()(target);
   };
 };
\ No newline at end of file
diff --git a/src/exception/utils.ts b/src/exception/utils.ts
index 539786e..d5c7512 100644
--- a/src/exception/utils.ts
+++ b/src/exception/utils.ts
@@ -4,35 +4,30 @@ import { EXCEPTION_FILTER_DEFAULT_SYMBOL, EXCEPTION_FILTER_MAP_INJECT_ID } from
 import { ArtusStdError } from './impl';
 import { ExceptionFilterMapType, ExceptionFilterType } from './types';
 
-export const matchExceptionFilter = (err: Error, container: Container): ExceptionFilterType | null => {
+export const matchExceptionFilterClazz =  (err: Error, container: Container): Constructable<ExceptionFilterType> | null => {
   const filterMap: ExceptionFilterMapType = container.get(EXCEPTION_FILTER_MAP_INJECT_ID, {
     noThrow: true,
   });
   if (!filterMap) {
     return null;
   }
-  let targetFilterClazz: Constructable<ExceptionFilterType>;
-  // handle ArtusStdError with code simply
+  let targetFilterClazz: Constructable<ExceptionFilterType> | null = null;
   if (err instanceof ArtusStdError) {
+    // handle ArtusStdError with code simply
     targetFilterClazz = filterMap.get(err.code);
-  }
-  if (!targetFilterClazz) {
-    // handler other Exception by Clazz
-    for (const errorClazz of filterMap.keys()) {
-      if (typeof errorClazz === 'string' || typeof errorClazz === 'symbol') {
-        continue;
-      }
-      if (err instanceof errorClazz) {
-        targetFilterClazz = filterMap.get(errorClazz);
-        break;
-      }
-    }
-  }
-  if (!targetFilterClazz && filterMap.has(EXCEPTION_FILTER_DEFAULT_SYMBOL)) {
+  } else if (filterMap.has(err['constructor'] as Constructable<Error>)) {
+    // handle CustomErrorClazz
+    targetFilterClazz = filterMap.get(err['constructor'] as Constructable<Error>);
+  } else if (filterMap.has(EXCEPTION_FILTER_DEFAULT_SYMBOL)) {
     // handle default ExceptionFilter
     targetFilterClazz = filterMap.get(EXCEPTION_FILTER_DEFAULT_SYMBOL);
   }
+  return targetFilterClazz;
+};
 
+export const matchExceptionFilter = (err: Error, container: Container): ExceptionFilterType | null => {
+  const filterClazz = matchExceptionFilterClazz(err, container);
+  
   // return the instance of exception filter
-  return targetFilterClazz ? container.get(targetFilterClazz) : null;
+  return filterClazz ? container.get(filterClazz) : null;
 };
diff --git a/src/loader/impl/exception_filter.ts b/src/loader/impl/exception_filter.ts
index cc1a591..6ea5017 100644
--- a/src/loader/impl/exception_filter.ts
+++ b/src/loader/impl/exception_filter.ts
@@ -5,7 +5,7 @@ import { ArtusStdError, EXCEPTION_FILTER_DEFAULT_SYMBOL, EXCEPTION_FILTER_MAP_IN
 import { Constructable } from '@artus/injection';
 import { ExceptionFilterMapType, ExceptionFilterType, ExceptionIdentifier } from '../../exception/types';
 
-@DefineLoader('exception_filter')
+@DefineLoader('exception-filter')
 class ExceptionFilterLoader extends ModuleLoader {
   async load(item: ManifestItem) {
     // Get or Init Exception Filter Map


Test patch:
diff --git a/test/fixtures/app_koa_with_ts/src/filter/default.ts b/test/fixtures/app_koa_with_ts/src/filter/default.ts
new file mode 100644
index 0000000..935ce05
--- /dev/null
+++ b/test/fixtures/app_koa_with_ts/src/filter/default.ts
@@ -0,0 +1,8 @@
+import { Catch, ExceptionFilterType } from '../../../../../src';
+
+@Catch()
+export class MockExceptionFilter implements ExceptionFilterType {
+  async catch(_err: Error): Promise<void> {
+    // Empty filter
+  }
+}
diff --git a/test/fixtures/exception_filter/bootstrap.ts b/test/fixtures/exception_filter/bootstrap.ts
index 465ace1..fa9239f 100644
--- a/test/fixtures/exception_filter/bootstrap.ts
+++ b/test/fixtures/exception_filter/bootstrap.ts
@@ -15,7 +15,7 @@ async function main() {
         path: path.resolve(__dirname, './filter'),
         extname: '.ts',
         filename: 'filter.ts',
-        loader: 'exception_filter',
+        loader: 'exception-filter',
         loaderState: {
           exportNames: [
             'TestDefaultExceptionHandler',
diff --git a/test/fixtures/exception_invalid_filter/bootstrap.ts b/test/fixtures/exception_invalid_filter/bootstrap.ts
index 8bb2b60..c7e910d 100644
--- a/test/fixtures/exception_invalid_filter/bootstrap.ts
+++ b/test/fixtures/exception_invalid_filter/bootstrap.ts
@@ -13,7 +13,7 @@ async function main() {
         path: path.resolve(__dirname, './filter'),
         extname: '.ts',
         filename: 'filter.ts',
-        loader: 'exception_filter',
+        loader: 'exception-filter',
         loaderState: {
           exportNames: [
             'TestInvalidFilter',
diff --git a/test/scanner.test.ts b/test/scanner.test.ts
index f93c1e0..7d1a2fc 100644
--- a/test/scanner.test.ts
+++ b/test/scanner.test.ts
@@ -13,20 +13,21 @@ describe('test/scanner.test.ts', () => {
     expect(manifest).toBeDefined();
     expect(manifest.items).toBeDefined();
     // console.log('manifest', manifest);
-    expect(manifest.items.length).toBe(10);
+    expect(manifest.items.length).toBe(11);
 
     expect(manifest.items.find(item => item.filename === 'not_to_be_scanned_file.ts')).toBeFalsy();
 
     expect(manifest.items.filter(item => item.loader === 'plugin-config').length).toBe(0);
     expect(manifest.items.filter(item => item.loader === 'plugin-meta').length).toBe(1);
     expect(manifest.items.filter(item => item.loader === 'exception').length).toBe(1);
+    expect(manifest.items.filter(item => item.loader === 'exception-filter').length).toBe(1);
     expect(manifest.items.filter(item => item.loader === 'lifecycle-hook-unit').length).toBe(2);
     expect(manifest.items.filter(item => item.loader === 'config').length).toBe(1);
     expect(manifest.items.filter(item => item.loader === 'module').length).toBe(4);
 
     expect(manifest.items.filter(item => item.unitName === 'redis').length).toBe(2);
     expect(manifest.items.filter(item => item.unitName === 'mysql').length).toBe(0);
-    expect(manifest.items.filter(item => item.source === 'app').length).toBe(8);
+    expect(manifest.items.filter(item => item.source === 'app').length).toBe(9);
     expect(manifest.pluginConfig).toStrictEqual({
       redis: { enable: true, path: path.join('src', 'redis_plugin') },
       mysql: { enable: false, path: path.join('src', 'mysql_plugin') },
@@ -37,7 +38,7 @@ describe('test/scanner.test.ts', () => {
     // console.log('devManifest', devManifest);
     expect(devManifest).toBeDefined();
     expect(devManifest.items).toBeDefined();
-    expect(devManifest.items.length).toBe(12);
+    expect(devManifest.items.length).toBe(13);
     expect(devManifest.items.filter(item => item.loader === 'config').length).toBe(2);
     expect(devManifest.items.filter(item => item.loader === 'plugin-meta').length).toBe(2);
     expect(devManifest.items.find(item => item.unitName === 'testDuplicate')).toBeDefined();