Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/websockets #455

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

feature/websockets #455

wants to merge 47 commits into from

Conversation

PaulDalek
Copy link
Contributor

@PaulDalek PaulDalek commented Dec 1, 2023

Added a websocket connection possibility, touch #456.

@PaulDalek PaulDalek linked an issue Dec 4, 2023 that may be closed by this pull request
Copy link
Contributor

@jszuminski jszuminski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

I'm not sure if that's an error, but when I try to run node ./bin/cli.js --startServer 1 --logLevel 4 with the following .env file:

WS_ENABLE=true
WS_URL=ws://localhost:7777

I get the following error:

ZOD Error related to ENV

file:///Users/jakubszuminski/Documents/Coding/Highcharts/exporting/node-export-server/node_modules/zod/lib/index.mjs:538
                const error = new ZodError(ctx.common.issues);
                              ^
ZodError: [
  {
    "expected": "'true' | 'false'",
    "received": "undefined",
    "code": "invalid_type",
    "path": [
      "WS_RECONNECT"
    ],
    "message": "Required"
  },
  {
    "expected": "'true' | 'false'",
    "received": "undefined",
    "code": "invalid_type",
    "path": [
      "WS_REJECT_UNAUTHORIZED"
    ],
    "message": "Required"
  },
  {
    "code": "invalid_type",
    "expected": "number",
    "received": "nan",
    "path": [
      "WS_PING_TIMEOUT"
    ],
    "message": "Expected number, received nan"
  },
  {
    "code": "invalid_type",
    "expected": "number",
    "received": "nan",
    "path": [
      "WS_RECONNECT_INTERVAL"
    ],
    "message": "Expected number, received nan"
  },
  {
    "code": "invalid_type",
    "expected": "string",
    "received": "undefined",
    "path": [
      "WS_SECRET"
    ],
    "message": "Required"
  }
]
    at Object.get error [as error] (file:///Users/jakubszuminski/Documents/Coding/Highcharts/exporting/node-export-server/node_modules/zod/lib/index.mjs:538:31)
    at ZodObject.parse (file:///Users/jakubszuminski/Documents/Coding/Highcharts/exporting/node-export-server/node_modules/zod/lib/index.mjs:638:22)
    at file:///Users/jakubszuminski/Documents/Coding/Highcharts/exporting/node-export-server/lib/envConfig.js:16:36
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12) {
  issues: [
    {
      expected: "'true' | 'false'",
      received: 'undefined',
      code: 'invalid_type',
      path: [ 'WS_RECONNECT' ],
      message: 'Required'
    },
    {
      expected: "'true' | 'false'",
      received: 'undefined',
      code: 'invalid_type',
      path: [ 'WS_REJECT_UNAUTHORIZED' ],
      message: 'Required'
    },
    {
      code: 'invalid_type',
      expected: 'number',
      received: 'nan',
      path: [ 'WS_PING_TIMEOUT' ],
      message: 'Expected number, received nan'
    },
    {
      code: 'invalid_type',
      expected: 'number',
      received: 'nan',
      path: [ 'WS_RECONNECT_INTERVAL' ],
      message: 'Expected number, received nan'
    },
    {
      code: 'invalid_type',
      expected: 'string',
      received: 'undefined',
      path: [ 'WS_SECRET' ],
      message: 'Required'
    }
  ],
  addIssue: [Function (anonymous)],
  addIssues: [Function (anonymous)],
  errors: [
    {
      expected: "'true' | 'false'",
      received: 'undefined',
      code: 'invalid_type',
      path: [ 'WS_RECONNECT' ],
      message: 'Required'
    },
    {
      expected: "'true' | 'false'",
      received: 'undefined',
      code: 'invalid_type',
      path: [ 'WS_REJECT_UNAUTHORIZED' ],
      message: 'Required'
    },
    {
      code: 'invalid_type',
      expected: 'number',
      received: 'nan',
      path: [ 'WS_PING_TIMEOUT' ],
      message: 'Expected number, received nan'
    },
    {
      code: 'invalid_type',
      expected: 'number',
      received: 'nan',
      path: [ 'WS_RECONNECT_INTERVAL' ],
      message: 'Expected number, received nan'
    },
    {
      code: 'invalid_type',
      expected: 'string',
      received: 'undefined',
      path: [ 'WS_SECRET' ],
      message: 'Required'
    }
  ]
}

Should all of those options be required? Or only some of them? What do you think about including default values for the users?

@PaulDalek
Copy link
Contributor Author

@jakubSzuminski Nice catch, corrected properties of the config object to optionals.

@PaulDalek PaulDalek marked this pull request as ready for review January 3, 2024 17:59
@PaulDalek PaulDalek changed the base branch from stable to master January 4, 2024 17:45
Copy link
Contributor

@jszuminski jszuminski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but shouldn't we add the EnvConfig zod parser to envs.js which has been added here: #498 so that we have all the envs & their validators in one file, in one place?

@PaulDalek
Copy link
Contributor Author

@jszuminski Yes but I will do it once the test/basic-test-env-setup branch is merged to the master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for WebSocket connection
2 participants