FoundKeyGang/FoundKey
FoundKeyGang/FoundKey
7
19
Fork 22
Code Issues 37 Pull requests 4 Releases Activity

allow redis family to be specified as a string #165

Manually merged
Johann150 merged 4 commits from redis-family-friendly-strings into main 2022-10-02 16:47:42 +00:00
Conversation 1 Commits 4 Files changed 6 +21 -6
3 changed files with 21 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 5a52532c99 - Show all commits

2
.config/example.yml
View file

@ -57,7 +57,7 @@ db:
redis:
host: localhost
port: 6379
#family: 0 # 0=Both, 4=IPv4, 6=IPv6
#family: dual # can be either a number or string (0/dual, 4/ipv4, 6/ipv6)
#pass: example-pass
#prefix: example-prefix
#db: 1

11
packages/backend/src/config/types.ts
View file

@ -1,5 +1,10 @@
/**
* ユーザーが設定する必要のある情報
* IP address family
*/
export type IpFamily = 'ipv4' | 'ipv6' | 'dual';
/**
* Configuration options set up by the user
*/
export type Source = {
repository_url?: string;
@ -19,7 +24,7 @@ export type Source = {
redis: {
host: string;
port: number;
family?: number;
family?: number | IpFamily;
pass: string;
db?: number;
prefix?: string;
@ -47,7 +52,7 @@ export type Source = {
id: string;
outgoingAddressFamily?: 'ipv4' | 'ipv6' | 'dual';
outgoingAddressFamily?: IpFamily;
deliverJobConcurrency?: number;
inboxJobConcurrency?: number;

14
packages/backend/src/db/redis.ts
View file

@ -1,11 +1,21 @@
import Redis from 'ioredis';
import config from '@/config/index.js';
import { IpFamily } from '@/config/types.js';
export function createConnection() {
function getRedisFamily(family?: IpFamily | number): number {
switch (family) {
case 'ipv4': return 4;
case 'ipv6': return 6;
case 'dual': return 0;
default: return family ?? 0;
norm marked this conversation as resolved Outdated
Johann150 commented 2022-09-21 10:14:39 +00:00
Outdated
Review
Copy link

This does not protect against someone entering another string value, e.g. IPv4 will cause an error, as will IPv7. We should probably have a nicer error or warning message (default to dual?) instead of this verbose ioredis error:

[ioredis] Unhandled error event: TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.family' must be one of: 0, 4, 6. Received 'IPv7'
    at lookup (node:dns:183:11)
    at node:net:1186:5
    at defaultTriggerAsyncIdScope (node:internal/async_hooks:464:18)
    at lookupAndConnect (node:net:1185:3)
    at Socket.connect (node:net:1113:5)
    at Object.connect (node:net:234:17)
    at /home/misskey/misskey/node_modules/ioredis/built/connectors/StandaloneConnector.js:58:45
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11)
[ioredis] Unhandled error event: TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.family' must be one of: 0, 4, 6. Received 'IPv7'
    at lookup (node:dns:183:11)
    at node:net:1186:5
    at defaultTriggerAsyncIdScope (node:internal/async_hooks:464:18)
    at lookupAndConnect (node:net:1185:3)
    at Socket.connect (node:net:1113:5)
    at Object.connect (node:net:234:17)
    at /home/misskey/misskey/node_modules/ioredis/built/connectors/StandaloneConnector.js:58:45
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11)
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.family' must be one of: 0, 4, 6. Received 'IPv7'
    at lookup (node:dns:183:11)
    at node:net:1186:5
    at defaultTriggerAsyncIdScope (node:internal/async_hooks:464:18)
    at lookupAndConnect (node:net:1185:3)
    at Socket.connect (node:net:1113:5)
    at Object.connect (node:net:234:17)
    at /home/misskey/misskey/node_modules/ioredis/built/connectors/StandaloneConnector.js:58:45
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11) {
  code: 'ERR_INVALID_ARG_VALUE'
}

Node.js v18.9.0
This does not protect against someone entering another string value, e.g. `IPv4` will cause an error, as will `IPv7`. We should probably have a nicer error or warning message (default to dual?) instead of this verbose ioredis error: ```text [ioredis] Unhandled error event: TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.family' must be one of: 0, 4, 6. Received 'IPv7' at lookup (node:dns:183:11) at node:net:1186:5 at defaultTriggerAsyncIdScope (node:internal/async_hooks:464:18) at lookupAndConnect (node:net:1185:3) at Socket.connect (node:net:1113:5) at Object.connect (node:net:234:17) at /home/misskey/misskey/node_modules/ioredis/built/connectors/StandaloneConnector.js:58:45 at process.processTicksAndRejections (node:internal/process/task_queues:77:11) [ioredis] Unhandled error event: TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.family' must be one of: 0, 4, 6. Received 'IPv7' at lookup (node:dns:183:11) at node:net:1186:5 at defaultTriggerAsyncIdScope (node:internal/async_hooks:464:18) at lookupAndConnect (node:net:1185:3) at Socket.connect (node:net:1113:5) at Object.connect (node:net:234:17) at /home/misskey/misskey/node_modules/ioredis/built/connectors/StandaloneConnector.js:58:45 at process.processTicksAndRejections (node:internal/process/task_queues:77:11) node:internal/process/promises:288 triggerUncaughtException(err, true /* fromPromise */); ^ TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.family' must be one of: 0, 4, 6. Received 'IPv7' at lookup (node:dns:183:11) at node:net:1186:5 at defaultTriggerAsyncIdScope (node:internal/async_hooks:464:18) at lookupAndConnect (node:net:1185:3) at Socket.connect (node:net:1113:5) at Object.connect (node:net:234:17) at /home/misskey/misskey/node_modules/ioredis/built/connectors/StandaloneConnector.js:58:45 at process.processTicksAndRejections (node:internal/process/task_queues:77:11) { code: 'ERR_INVALID_ARG_VALUE' } Node.js v18.9.0 ```
Michcio commented 2022-09-21 15:30:47 +00:00
Outdated
Review
Copy link

Do we have any form of validation of the yaml config outside of the few asserts at start time? We could employ ajv here since it's already used in the project (assuming anyone other than me is a fan of it)

Do we have any form of validation of the yaml config outside of the few asserts at start time? We could employ ajv here since it's already used in the project (assuming anyone other than me is a fan of it)
norm commented 2022-09-21 15:41:54 +00:00
Outdated
Review
Copy link

AFAIK there's not a whole lot of validation of the config, that error would have also happened with the current code in main as well.

AFAIK there's not a whole lot of validation of the config, that error would have also happened with the current code in main as well.
}
}
export function createConnection(): Redis.Redis {
return new Redis({
port: config.redis.port,
host: config.redis.host,
family: config.redis.family ?? 0,
family: getRedisFamily(config.redis.family),
password: config.redis.pass,
keyPrefix: `${config.redis.prefix}:`,
db: config.redis.db || 0,