Way to disable message content being logged

gaurav512 commented 7 months ago

Hi,

I am looking for a way to never output / always hide the message contents from the application logs. If such way already exists, I would like to know how.

Use-case

I am building an app which contains sensitive content in the messages that I do not want to be logged in the application logs.

My LogLevel is 'INFO'. I recently saw some API failure logs in my application, which in turn resulted in the message data being logged in the app logs.

Error: A request error occurred: read ECONNRESET
    at requestErrorWithOriginal (/app/node_modules/@slack/web-api/dist/errors.js:33:33)
    at /app/node_modules/@slack/web-api/dist/WebClient.js:433:65
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async run (/app/node_modules/@slack/web-api/node_modules/p-queue/dist/index.js:163:29) {
  code: 'slack_webapi_request_error',
  original: AxiosError: read ECONNRESET
      at Function.AxiosError.from (/app/node_modules/@slack/web-api/node_modules/axios/dist/node/axios.cjs:837:14)
      at ClientRequest.handleRequestError (/app/node_modules/@slack/web-api/node_modules/axios/dist/node/axios.cjs:3083:25)
      at ClientRequest.emit (node:events:513:28)
      at TLSSocket.socketErrorListener (node:_http_client:494:9)
      at TLSSocket.emit (node:events:513:28)
      at emitErrorNT (node:internal/streams/destroy:157:8)
      at emitErrorCloseNT (node:internal/streams/destroy:122:3)
      at processTicksAndRejections (node:internal/process/task_queues:83:21) {
    syscall: 'read',
    code: 'ECONNRESET',
    errno: -104,
    config: {
      transitional: [Object],
      adapter: [Array],
      transformRequest: [Array],
      transformResponse: [Array],
      timeout: 0,
      xsrfCookieName: 'XSRF-TOKEN',
      xsrfHeaderName: 'X-XSRF-TOKEN',
      maxContentLength: -1,
      maxBodyLength: -1,
      env: [Object],
      validateStatus: [Function: validateStatus],
      headers: [Object [AxiosHeaders]],
      baseURL: 'https://slack.com/api/',
      maxRedirects: 0,
      proxy: false,
      method: 'post',
      url: 'views.publish',
      data: <MY MESSAGE DATA>
      ...
      ...

Reproducible in:

The Slack SDK version

"slack/bolt": "^3.13.3",
"slack/logger": "^3.0.0",
"slack/web-api": "^6.9.1"

Node.js runtime version

v16.8.1

OS info

Red Hat Enterprise Linux 8

Steps to reproduce:

App error logs (in case of internal failures) contain message data in the logs

Expected result:

Application logs contain message data.

Actual result:

I want to exclude this message data from the application logs.

Requirements

For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. :bow:

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

seratch commented 7 months ago

Thanks for reporting this. This could be improved on the underlying web-api package side (https://github.com/slackapi/node-slack-sdk/blob/%40slack/web-api%407.0.2/packages/web-api/src/errors.ts#L79), so let me transfer this to node-slack-sdk repo.

Parama92 commented 5 months ago

Hi! New contributor here 👋

Upon reviewing this issue, I noticed that -

The Error type - WebAPIRequestError (the error shared in the issue description), contains a property called original which holds the raw error received from axios
The config property within this original is a part of the error data received from axios. However, its presence doesn't seem to depend on any configuration (as evident here) that can be set while creating the axios client here.

Potential solution -

Making the original property optional would be the easiest solution, although this might result in losing some crucial details about the initiated request.
Alternatively, removing the data property from original.config could address the issue, albeit with the caveat of relying on another library to maintain its contract.

In my opinion, opting for the first solution seems most sensible. What are your thoughts on this approach?

seratch commented 5 months ago

Thanks for the comment! I agree that the simplest solution would be to provide an opt-out option, allowing for the removal of the original property. Although I haven't explored in depth, adding a flag option like attachOriginalToWebAPIRequestError? : boolean to WebClientOptions should work well. If the property doesn't exist, the flag should default to true for the consistency with existing behavior. The naming may sound somewhat verbose, but I believe being specific here should not be so bad.

slackapi / node-slack-sdk