Fix stacktrace enhancement for errors in pipes (#2317)

## What does this PR do ? Work had been done on stacktrace a while ago to enhance them so developers can distinguish there code from Kuzzle one. (See #1944) Few problems remains: - `hilightUserCode` was called many times on the same stack - errors thrown by the SDK in a pipe did not include the line with the code that triggered the error This fix cover the following additional use cases: ``` app.pipe.register('server:afterNow', async req => { await app.sdk.collection.list('do-not-exist'); }); app.pipe.register('server:afterInfo', async req => { throw new BadRequestError('afterInfo'); }); app.pipe.register('server:afterGetStats', async function afterGetStats (req) { await app.sdk.collection.list('do-not-exist'); }); app.pipe.register('server:afterMetrics', async function afterMetrics (req) { throw new BadRequestError('afterMetrics'); }); ``` ### Other changes - remove useless await in FunnelProtocol + convert to Typescript - extract `removeErrorStack` into `util` directory
kuzzleio · Apr 11, 2022 · 4ce6c97 · 4ce6c97
1 parent 7a53414
commit 4ce6c97
Show file tree

Hide file tree

Showing 15 changed files with 204 additions and 133 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -97,3 +97,27 @@ lib/types/config/ServicesConfiguration.js
 lib/types/config/StorageService/StorageServiceElasticsearchConfiguration.js
 lib/types/config/internalCache/InternalCacheRedisConfiguration.js
 lib/types/config/publicCache/PublicCacheRedisConfiguration.js
+lib/api/openapi/OpenApiManager.js
+lib/api/openapi/components/document/index.js
+lib/api/openapi/components/index.js
+lib/api/openapi/openApiGenerator.js
+lib/core/backend/backendErrors.js
+lib/core/backend/backendOpenApi.js
+lib/core/security/profileRepository.js
+lib/core/shared/sdk/funnelProtocol.js
+lib/kerror/index.js
+lib/model/security/profile.js
+lib/model/security/role.js
+lib/model/security/user.js
+lib/types/ControllerRights.js
+lib/types/HttpStream.js
+lib/types/OpenApiDefinition.js
+lib/types/Policy.js
+lib/types/PolicyRestrictions.js
+lib/types/Target.js
+lib/types/config/storageEngine/StorageEngineElasticsearchConfiguration.js
+lib/types/errors/ErrorDefinition.js
+lib/types/errors/ErrorDomains.js
+lib/util/array.js
+lib/util/bufferedPassThrough.js
+lib/util/dump-collection.js
diff --git a/.gitignore b/.gitignore
@@ -180,3 +180,27 @@ lib/types/config/ServicesConfiguration.js
 lib/types/config/StorageService/StorageServiceElasticsearchConfiguration.js
 lib/types/config/internalCache/InternalCacheRedisConfiguration.js
 lib/types/config/publicCache/PublicCacheRedisConfiguration.js
+lib/api/openapi/OpenApiManager.js
+lib/api/openapi/components/document/index.js
+lib/api/openapi/components/index.js
+lib/api/openapi/openApiGenerator.js
+lib/core/backend/backendErrors.js
+lib/core/backend/backendOpenApi.js
+lib/core/security/profileRepository.js
+lib/core/shared/sdk/funnelProtocol.js
+lib/kerror/index.js
+lib/model/security/profile.js
+lib/model/security/role.js
+lib/model/security/user.js
+lib/types/ControllerRights.js
+lib/types/HttpStream.js
+lib/types/OpenApiDefinition.js
+lib/types/Policy.js
+lib/types/PolicyRestrictions.js
+lib/types/Target.js
+lib/types/config/storageEngine/StorageEngineElasticsearchConfiguration.js
+lib/types/errors/ErrorDefinition.js
+lib/types/errors/ErrorDomains.js
+lib/util/array.js
+lib/util/bufferedPassThrough.js
+lib/util/dump-collection.js
diff --git a/lib/core/network/entryPoint.js b/lib/core/network/entryPoint.js
@@ -33,7 +33,7 @@ const MqttProtocol = require('./protocols/mqttProtocol');
 const InternalProtocol = require('./protocols/internalProtocol');
 const HttpWsProtocol = require('./protocols/httpwsProtocol');
 const Manifest = require('./protocolManifest');
-const removeErrorStack = require('./removeErrorStack');
+const { removeStacktrace } = require('../../util/stackTrace');
 const kerror = require('../../kerror');
 const { AccessLogger } = require('./accessLogger');
 
@@ -256,7 +256,7 @@ class EntryPoint {
 
       const response = _res.response.toJSON();
 
-      cb(removeErrorStack(response));
+      cb(removeStacktrace(response));
     });
   }
 
@@ -298,7 +298,7 @@ class EntryPoint {
   // --------------------------------------------------------------------
 
   _broadcast (data) {
-    const sanitized = removeErrorStack(data);
+    const sanitized = removeStacktrace(data);
 
     debug('[server] broadcasting data through all protocols: %a', sanitized);
 
@@ -316,7 +316,7 @@ class EntryPoint {
     request.setError(networkError.get('shutting_down'));
     this.logAccess(connection, request);
 
-    cb(removeErrorStack(request.response.toJSON()));
+    cb(removeStacktrace(request.response.toJSON()));
   }
 
   _notify (data) {
@@ -332,7 +332,7 @@ class EntryPoint {
     }
 
     try {
-      this.protocols.get(client.protocol).notify(removeErrorStack(data));
+      this.protocols.get(client.protocol).notify(removeStacktrace(data));
     }
     catch (e) {
       global.kuzzle.log.error(`[notify] protocol ${client.protocol} failed: ${e.message}`);

diff --git a/lib/core/network/protocols/httpwsProtocol.js b/lib/core/network/protocols/httpwsProtocol.js
@@ -32,7 +32,7 @@ const { Request } = require('../../../api/request');
 const { KuzzleError } = require('../../../kerror/errors');
 const Protocol = require('./protocol');
 const ClientConnection = require('../clientConnection');
-const removeErrorStack = require('../removeErrorStack');
+const { removeStacktrace } = require('../../../util/stackTrace');
 const debug = require('../../../util/debug');
 const kerror = require('../../../kerror');
 const HttpMessage = require('./httpMessage');
@@ -412,7 +412,7 @@ class HttpWsProtocol extends Protocol {
     // If a requestId is provided we use it instead of the generated one
     request.id = requestId || request.id;
 
-    const sanitized = removeErrorStack(request.response.toJSON()).content;
+    const sanitized = removeStacktrace(request.response.toJSON()).content;
 
     this.wsSend(socket, Buffer.from(JSON.stringify(sanitized)));
   }
@@ -831,7 +831,7 @@ class HttpWsProtocol extends Protocol {
       ? error
       : kerrorHTTP.getFrom(error, 'unexpected_error', error.message);
 
-    const content = Buffer.from(JSON.stringify(removeErrorStack(kerr)));
+    const content = Buffer.from(JSON.stringify(removeStacktrace(kerr)));
 
     debugHTTP('[%s] httpSendError: %a', message.connection.id, kerr);
 
@@ -884,7 +884,7 @@ class HttpWsProtocol extends Protocol {
    * @returns {Buffer}
    */
   httpRequestToResponse (request, message) {
-    let data = removeErrorStack(request.response.toJSON());
+    let data = removeStacktrace(request.response.toJSON());
 
     if (message.requestId !== data.requestId) {
       data.requestId = message.requestId;

diff --git a/lib/core/network/protocols/mqttProtocol.js b/lib/core/network/protocols/mqttProtocol.js
@@ -22,12 +22,13 @@
 'use strict';
 
 const net = require('net');
+
 const aedes = require('aedes');
 
 const ClientConnection = require('../clientConnection');
 const Protocol = require('./protocol');
 const { Request } = require('../../../api/request');
-const removeErrorStack = require('../removeErrorStack');
+const { removeStacktrace } = require('../../../util/stackTrace');
 const kerror = require('../../../kerror').wrap('network', 'mqtt');
 const debug = require('../../../util/debug')('kuzzle:network:protocols:mqtt');
 
@@ -238,7 +239,7 @@ class MqttProtocol extends Protocol {
       connection,
       error: kerror.getFrom(error, 'unexpected_error', error.message)
     });
-    this._respond(client, removeErrorStack(errReq.response.toJSON()));
+    this._respond(client, removeStacktrace(errReq.response.toJSON()));
   }
 
   _authorizePublish (client, packet, callback) {

diff --git a/lib/core/network/removeErrorStack.js b/lib/core/network/removeErrorStack.js
diff --git a/lib/core/network/router.js b/lib/core/network/router.js
@@ -24,7 +24,7 @@
 const { Request } = require('../../api/request');
 const kerror = require('../../kerror');
 const HttpRouter = require('./httpRouter');
-const removeErrorStack = require('./removeErrorStack');
+const { removeStacktrace } = require('../../util/stackTrace');
 
 /**
  * @class Router
@@ -197,7 +197,7 @@ class Router {
             _res.setError(err);
           }
 
-          cb(removeErrorStack(_res));
+          cb(removeStacktrace(_res));
         });
       }
     });

diff --git a/lib/core/shared/sdk/embeddedSdk.ts b/lib/core/shared/sdk/embeddedSdk.ts
@@ -29,7 +29,7 @@ import {
 } from 'kuzzle-sdk';
 
 import { RequestPayload, ResponsePayload } from '../../../types';
-import FunnelProtocol from './funnelProtocol';
+import { FunnelProtocol } from './funnelProtocol';
 import { isPlainObject } from '../../../util/safeObject';
 import * as kerror from '../../../kerror';
 import ImpersonatedSDK from './impersonatedSdk';
@@ -94,7 +94,8 @@ export class EmbeddedSDK extends Kuzzle {
   realtime: EmbeddedRealtime;
 
   constructor () {
-    super(new FunnelProtocol(), { autoResubscribe: false });
+    // FunnelProtocol is not technically a valid SDK protocol
+    super(new FunnelProtocol() as any, { autoResubscribe: false });
   }
 
   /**

diff --git a/lib/core/shared/sdk/funnelProtocol.js → lib/core/shared/sdk/funnelProtocol.ts b/lib/core/shared/sdk/funnelProtocol.js → lib/core/shared/sdk/funnelProtocol.ts
@@ -19,21 +19,19 @@
  * limitations under the License.
  */
 
-'use strict';
+import { KuzzleEventEmitter } from 'kuzzle-sdk';
 
-const { KuzzleEventEmitter } = require('kuzzle-sdk');
+import { RequestPayload } from '../../../types';
+import { Request } from '../../../api/request';
+import * as kerror from '../../../kerror';
 
-const { Request } = require('../../../api/request');
-const kerror = require('../../../kerror');
-const { hilightUserCode } = require('../../../util/stackTrace');
+export class FunnelProtocol extends KuzzleEventEmitter {
+  private id = 'funnel';
+  private connectionId: string = null;
 
-class FunnelProtocol extends KuzzleEventEmitter {
   constructor () {
     super();
 
-    this.id = 'funnel';
-    this.connectionId = null;
-
     /**
      * Realtime notifications are sent by the InternalProtocol
      * through the internal event system.
@@ -51,7 +49,7 @@ class FunnelProtocol extends KuzzleEventEmitter {
   /**
    *  Hydrate the user and execute SDK query
    */
-  async query (request) {
+  async query (request: RequestPayload) {
     if (! this.connectionId) {
       this.connectionId = await global.kuzzle.ask('core:network:internal:connectionId:get');
     }
@@ -75,31 +73,21 @@ class FunnelProtocol extends KuzzleEventEmitter {
 
     const kuzzleRequest = new Request(request, requestOptions);
 
-    try {
-      if (requestOptions.user && request.__checkRights__
-        && ! await requestOptions.user.isActionAllowed(kuzzleRequest)
-      ) {
-        throw kerror.get(
-          'security',
-          'rights',
-          'forbidden',
-          kuzzleRequest.input.controller,
-          kuzzleRequest.input.action,
-          requestOptions.user._id);
-      }
-
-      const result = await global.kuzzle.funnel.executePluginRequest(kuzzleRequest);
-
-      return { result };
+    if ( requestOptions.user
+      && request.__checkRights__
+      && ! await requestOptions.user.isActionAllowed(kuzzleRequest)
+    ) {
+      throw kerror.get(
+        'security',
+        'rights',
+        'forbidden',
+        kuzzleRequest.input.controller,
+        kuzzleRequest.input.action,
+        requestOptions.user._id);
     }
-    catch (error) {
-      if (error.stack) {
-        error.stack = error.stack.split('\n').map(hilightUserCode).join('\n');
-      }
 
-      throw error;
-    }
+    const result = await global.kuzzle.funnel.executePluginRequest(kuzzleRequest);
+
+    return { result };
   }
 }
-
-module.exports = FunnelProtocol;
diff --git a/lib/kerror/index.ts b/lib/kerror/index.ts
@@ -26,7 +26,6 @@ import { JSONObject } from 'kuzzle-sdk';
 
 import { domains as internalDomains } from './codes';
 import * as errors from './errors';
-import { hilightUserCode } from '../util/stackTrace';
 import { KuzzleError } from './errors';
 import { ErrorDefinition, ErrorDomains } from '../types';
 
@@ -130,8 +129,7 @@ function cleanStackTrace (error: KuzzleError): void {
 
       // filter all lines related to the kerror object
       return ! line.includes(currentFileName);
-    })
-    .map(hilightUserCode);
+    });
 
   // insert a deletion message in place of the new error instantiation line
   newStack[messageLength] = '      [...Kuzzle internal calls deleted...]';

diff --git a/lib/types/KuzzleDocument.ts b/lib/types/KuzzleDocument.ts
@@ -1,6 +1,8 @@
 import { JSONObject } from 'kuzzle-sdk';
 
-// Should be in the SDK instead
+/**
+ * @deprecated Use KDocument instead (See https://docs.kuzzle.io/sdk/js/7/essentials/strong-typing/)
+ */
 export interface KuzzleDocument {
   _id: string;