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

ioredis support #22

Closed
directormac opened this issue Jun 27, 2024 · 10 comments
Closed

ioredis support #22

directormac opened this issue Jun 27, 2024 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@directormac
Copy link

Just followed the read me and what was suggested by @MathurAditya724 .

and in reference to #10

Reproducible snippet

import Redis from 'ioredis';
import { env } from './env.util';
import { rateLimiter } from 'hono-rate-limiter';
import { generateId } from './crypto.util';

import { type Store } from 'hono-rate-limiter';
import { RedisStore } from 'rate-limit-redis';

const client = new Redis(env.REDIS_URL);

const limiter = rateLimiter({
	windowMs: 15 * 60 * 1000,
	limit: 100,
	// Change the header according to your proxy
	keyGenerator: (c) => c.req.header('X-Forwarded-for') + generateId(20),
	store: new RedisStore({
		sendCommand: (...args: string[]) => client.sendCommand(args)
	}) as unknown as Store // Adding the correct type
});

export { client, limiter };

Type error is
image

@MathurAditya724 MathurAditya724 self-assigned this Jun 27, 2024
@MathurAditya724 MathurAditya724 added the bug Something isn't working label Jun 27, 2024
@MathurAditya724
Copy link
Member

Hey @directormac, apologies that it took me some time to go through the issue, but upon close inspection, the problem isn't in hono-rate-limiter but in rate-limit-redis and ioredis types. To resolve it you can write sendCommand: (...args: string[]) => client.sendCommand(args) as Promise<RedisReply>, because out of the box ioredis returns unknown.

You can check the reference here -
redis-rate-limiter - https://github.com/express-rate-limit/rate-limit-redis/blob/main/source/types.ts#L14
ioredis - https://github.com/redis/ioredis/blob/main/lib/Redis.ts#L423

@directormac
Copy link
Author

Hi @MathurAditya724

so here is the new snippet according to that

const limiter = rateLimiter({
	windowMs: 15 * 60 * 1000,
	limit: 100,
	keyGenerator: (c) => c.req.header("X-Forwarded-for") + generateId(20),
	store: new RedisStore({
		sendCommand: (...args) => client.sendCommand(args) as Promise<RedisReply>,
	}) as unknown as Store,
});

export { client, limiter };

Only wrong args being passed now would it be safe to assume that the args are commands? and just cast it like this

client.sendCommand(args as unknown as Command) as Promise<RedisReply>,

@MathurAditya724
Copy link
Member

Yes, I also don't like these semi-typed functions, but this API is neither created nor maintained by me so can't change that. This usage is given here https://github.com/express-rate-limit/rate-limit-redis/tree/main?tab=readme-ov-file#examples, and honesty I went through the code base when I was building @hono-rate-limiter/redis package, it's safe you don't have to worry about it.

@directormac
Copy link
Author

You should also put this in the examples incase someone needs it and close this issue.

@MathurAditya724
Copy link
Member

Yes, I am thinking the same

@directormac
Copy link
Author

Solution

const limiter = rateLimiter({
	windowMs: 15 * 60 * 1000,
	limit: 100,
	keyGenerator: (c) => c.req.header("X-Forwarded-for") + generateId(20),
	store: new RedisStore({
		sendCommand: (...args) => client.sendCommand(args as unknown as Command) as Promise<RedisReply>,
	}) as unknown as Store,
});

export { client, limiter };

@csulit
Copy link

csulit commented Aug 16, 2024

Object literal may only specify known properties, and 'sendCommand' does not exist in type 'Options'.ts(2353)

something change?

@real2two
Copy link

real2two commented Aug 31, 2024

Object literal may only specify known properties, and 'sendCommand' does not exist in type 'Options'.ts(2353)

something change?

+1 The solution above doesn't work for me either. (Object literal may only specify known properties, and 'sendCommand' does not exist in type 'Options'. ts(2353))

EDIT: I realized I was looking at the wrong library (@hono-rate-limiter/redis) when I was supposed to use rate-limit-redis instead.

import { rateLimiter, type Store } from "hono-rate-limiter";
import { RedisStore, type RedisReply } from "rate-limit-redis";

const store = new RedisStore({
  sendCommand: (...args) => {
    return client.call(...args) as Promise<RedisReply>;
  },
}) as unknown as Store;

This still doesn't work as intended either though, because the rate limit seems to never expire (windowMs). I set the windowMs to 1 and I keep getting 429 (Too Many Requests) indefinetely.

@MathurAditya724
Copy link
Member

I am not sure why you would be putting 1 as a value in windowMs, Try putting something like 1 * 60 * 1000 (1 minute). Cause based on your setup the IO might take some time to update the values.

@MathurAditya724
Copy link
Member

Object literal may only specify known properties, and 'sendCommand' does not exist in type 'Options'.ts(2353)

something change?

Can you share a reproduction or your code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants