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

Introduce Bun.redis - a builtin Redis client for Bun #18812

Merged
merged 51 commits into from
Apr 8, 2025
Merged

Conversation

Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Apr 6, 2025

What does this PR do?

import { redis } from "bun";

// Set a key
await redis.set("greeting", "Hello from Bun!");

// Get a key
const greeting = await redis.get("greeting");
console.log(greeting); // "Hello from Bun!"

// Increment a counter
await redis.set("counter", "0");
await redis.incr("counter");

// Check if a key exists
const exists = await redis.exists("greeting");

// Delete a key
await redis.del("greeting");

fixes #18112

How did you verify your code works?

There are tests. Need to setup a managed redis before the CI will pass and work on the tests some more.

@robobun

This comment was marked as outdated.

this.reconnect_timer.state = .FIRED;

// Increment ref to ensure 'this' stays alive throughout the function
this.ref();
Copy link
Member

Choose a reason for hiding this comment

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

this is not necessary since reconnect already do ref/unref

const handshake_success = if (success == 1) true else false;
this.ref();
defer this.deref();
if (handshake_success) {
Copy link
Member

Choose a reason for hiding this comment

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

If handshake_success is false we need to fail this.client.failWithJSValue(this.globalObject, ssl_error.toJS(this.globalObject)); because it means the server rejected the connection

Comment on lines +1243 to +1249
const valkey = JSC.API.Valkey.create(globalThis, &[_]JSValue{.undefined}) catch |err| {
if (err != error.JSError) {
_ = globalThis.throwError(err, "Failed to create Redis client") catch {};
return .zero;
}
return .zero;
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const valkey = JSC.API.Valkey.create(globalThis, &[_]JSValue{.undefined}) catch |err| {
if (err != error.JSError) {
_ = globalThis.throwError(err, "Failed to create Redis client") catch {};
return .zero;
}
return .zero;
};
const valkey = JSC.API.Valkey.create(globalThis, &[_]JSValue{.undefined}) catch |err| switch (err) {
error.JSError => return .zero,
error.OutOfMemory => return global.throwOutOfMemoryValue(),
};

Copy link
Member

@dylan-conway dylan-conway Apr 8, 2025

Choose a reason for hiding this comment

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

would be nice if these getters had JSError!JSValue return types

this.client.socket = _socket(socket);

// Do not allow half-open connections
socket.close(.normal);
Copy link
Member

@cirospaciari cirospaciari Apr 8, 2025

Choose a reason for hiding this comment

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

We dont need to call close here unless we pass LIBUS_SOCKET_ALLOW_HALF_OPEN in options when connecting because we will call the on_end and close in the sequence, but should be a no-op

@Jarred-Sumner Jarred-Sumner marked this pull request as ready for review April 8, 2025 10:33
@Jarred-Sumner Jarred-Sumner merged commit ec87a27 into main Apr 8, 2025
69 of 71 checks passed
@Jarred-Sumner Jarred-Sumner deleted the jarred/redis2 branch April 8, 2025 10:34
@mckoda09
Copy link

mckoda09 commented Apr 8, 2025

thanks bro

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

Successfully merging this pull request may close these issues.

Bun KV
5 participants