Skip to content

Not working as expected in rainy days #2237

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

Closed
idaamit opened this issue Aug 18, 2022 · 5 comments · Fixed by #2226
Closed

Not working as expected in rainy days #2237

idaamit opened this issue Aug 18, 2022 · 5 comments · Fixed by #2226
Labels

Comments

@idaamit
Copy link

idaamit commented Aug 18, 2022

On redis connection failure, I do not get exception.
I have upgraded to redis 4.2.0 with the following lambda :

export const handler = async (
    event: APIGatewayEvent, context: Context, callback: APIGatewayProxyCallback
) => {
    try {
        console.log(`Lambda triggered with event: ${JSON.stringify(event)}`);
        context.callbackWaitsForEmptyEventLoop = false;
        const redisClient = redis.createClient({
            socket: {
                host: '127.0.0.1',
                port: 6379,
                reconnectStrategy: function (retriesAttemptedSoFar: number): number | Error {
                    if (retriesAttemptedSoFar > 3) {
                        // End reconnecting with error
                        return new Error(`Redis retry attempt=${retriesAttemptedSoFar}`);
                    }
                    console.info(` try reconnect after ${Math.min(retriesAttemptedSoFar * 10, 30)}ms`);
                    // reconnect after
                    return Math.min(retriesAttemptedSoFar * 10, 30);
                }
            },
        });
        redisClient.on('ready', () => {
            console.log(`Got "ready" event from Redis. Connection established.`);
        });
        redisClient.on('connect', () => {
            console.log(`Got "connect" event from Redis. Stream is connected to the server`);
        });
        redisClient.on('error', (err) => {
            console.log(`Got an ERROR from Redis. Details: ${err.message}`);
        });
        await redisClient.connect();
        let serverStates : string[] = [];
        const MAX_RETRIES = 3;
        let i = 0;
        while (i < MAX_RETRIES) {
            try {
                serverStates = await redisClient.lRange('123', 0, -1);
            } catch (e) {
                i++;
                const logMessage = i < MAX_RETRIES ?
                    `attempt ${i}: Failed to store message. Trying again...` :
                    `attempt ${i}: Failed to store message. All attempts failed`;
                console.warn(logMessage);
                await new Promise(resolve => setTimeout(resolve, 10));
            }
        }
        if (i >= MAX_RETRIES) {
            throw new Error('Exceeded Max Attempts to store message in Redis');
        }
        console.log(`ServerStates: ${JSON.stringify(serverStates)}`);

        console.log(`${JSON.stringify(event)} Lambda finished successfully`);
        callback(null, {
            statusCode: 200,
            body: 'theResult'
        });
    } catch (e) {
        const error = e as Error;
        console.error(`Lambda execution failed. Got error: ${error.message}`);
        callback(error);
    }
}
  • If I do not get exception from redis, all works.
  • If I do not specify reconnectStrategy and redis connection fail, the code trys to reconnect inifinitly.
  • If redis connection fail with reconnectStrategy code, lambda gets to the catch section after 3 re-connections as expected.

The problem is : If I succeed to connect to redis, but before lrage redis stops (redis connection fail Socket closed unexpectedly) redis client reconnects for 3 times as written in reconnectStrategy, and after the 3 re-connects, the code stops and do not get to the catch section, ending the lambda as successfully finished.

How can I get to the exception section in this scenario ?
In redis 3.1.X, in similar case lambda finished with exception.

Environment:

  • Node.js Version: v14.1.0
  • Redis Server Version: 6.2.6
  • Node Redis Version: 4.2.0
  • Platform: Windows 10
@idaamit idaamit added the Bug label Aug 18, 2022
@idaamit
Copy link
Author

idaamit commented Aug 18, 2022

summary of error:

  1. I have error handling for everything. with logs.
  2. after successful connect and before await redisClient.lRange('123', 0, -1); (I have "await"!!!!), I stop redis server locally and expect the reconnectStrategy to be triggered inside lRange function.
  3. I do NOT turn on redis server locally.
  4. expected behavior: I am expecting the reconnectStrategy to fail eventually and propagate the error to the await redisClient.lRange('123', 0, -1); (await !!!!).
  5. actual behavior: the error is not propagated to the catch of the await redisClient.lRange('123', 0, -1); and just thrown to the event loop and the process ends gracefully (exit code = 0 !!!! instead of 1").

@leibale
Copy link
Contributor

leibale commented Aug 18, 2022

Hi @idaamit, thanks for reporting it
I've already implemented a fix in #2226, wanna give it a shot (You'll need to clone my fork, checkout to reconnect-strategy-errors branch, and run npm i && npm run build-all) before I'll merge and release it?

leibale added a commit that referenced this issue Aug 22, 2022
* handle errors in reconnect strategy

* add test

* fix retries typo

* fix #2237 - flush queues on reconnect strategy error

* Update socket.ts

* Update socket.ts
leibale added a commit to leibale/node-redis that referenced this issue Aug 22, 2022
* handle errors in reconnect strategy

* add test

* fix retries typo

* fix redis#2237 - flush queues on reconnect strategy error

* Update socket.ts

* Update socket.ts
@idaamit
Copy link
Author

idaamit commented Aug 23, 2022

Hi @leibale ,
I have downloaded version 4.3.0 and the problem is solved, thank you for attending the issue so rapidly.

For testing, I was using redis-mock library, but because of the braking changes in version 4.x the library is no longer usable, I have opened an issue on it but I see repository is not maintained :-(.
@leibale do you have a different recommendation for mocking library?

@JonhnyDev
Copy link

From a few seconds the server goes down as in a container, then tried to downgrade to previous versions within 4.+ because of the same local error not 4.0.0 is only stable without errors in version 3.1.2.

@emilsivervik
Copy link

Hi @leibale , I have downloaded version 4.3.0 and the problem is solved, thank you for attending the issue so rapidly.

For testing, I was using redis-mock library, but because of the braking changes in version 4.x the library is no longer usable, I have opened an issue on it but I see repository is not maintained :-(. @leibale do you have a different recommendation for mocking library?

@idaamit If you're running Docker https://www.npmjs.com/package/testcontainers might be worth a try since then you could also use a proper watch implementation

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

Successfully merging a pull request may close this issue.

4 participants