Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions packages/backend/src/common/redis.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { EventEmitter } from 'events';
import { Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';

// A stand-in for ioredis: an event emitter with a connect() the test controls.
const instances: FakeRedis[] = [];
class FakeRedis extends EventEmitter {
status = 'wait';
constructor(
public url: string,
public opts: unknown,
) {
super();
instances.push(this);
}
connect = jest.fn(async () => {
throw new Error('connect ECONNREFUSED 127.0.0.1:6379');
});
}
jest.mock('ioredis', () => ({ __esModule: true, default: FakeRedis }));

import { RedisService } from './redis.service';

const config = (env: Record<string, string | undefined>) =>
({ get: (key: string) => env[key] }) as unknown as ConfigService;

describe('RedisService', () => {
let warn: jest.SpyInstance;

beforeEach(() => {
instances.length = 0;
warn = jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
jest.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined);
});

afterEach(() => jest.restoreAllMocks());

it('creates no client and connects nowhere when REDIS_URL is not set', async () => {
const svc = new RedisService(config({}));
await svc.onModuleInit();

expect(instances).toHaveLength(0);
expect(svc.isConfigured).toBe(false);
expect(svc.isConnected).toBe(false);
expect(await svc.get('k')).toBeNull();
expect(await svc.incr('k')).toBe(0);
await expect(svc.onModuleDestroy()).resolves.toBeUndefined();
expect(warn).not.toHaveBeenCalled();
});

it('treats a blank REDIS_URL as not set', async () => {
const svc = new RedisService(config({ REDIS_URL: ' ' }));
await svc.onModuleInit();
expect(instances).toHaveLength(0);
});

it('logs a repeated connection error once, and again after a recovery', async () => {
const svc = new RedisService(config({ REDIS_URL: 'redis://redis:6379' }));
await svc.onModuleInit();

expect(instances).toHaveLength(1);
expect(instances[0].url).toBe('redis://redis:6379');
expect(svc.isConfigured).toBe(true);
warn.mockClear(); // the failed first connect() is reported on its own

const refused = new Error('connect ECONNREFUSED 10.0.0.5:6379');
for (let i = 0; i < 5; i++) instances[0].emit('error', refused);
expect(warn).toHaveBeenCalledTimes(1);

instances[0].emit('ready');
instances[0].emit('error', refused);
expect(warn).toHaveBeenCalledTimes(2);
});
});
43 changes: 32 additions & 11 deletions packages/backend/src/common/redis.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,42 @@ import Redis from 'ioredis';
@Injectable()
export class RedisService implements OnModuleInit, OnModuleDestroy {
private readonly logger = new Logger(RedisService.name);
private client: Redis;
private client: Redis | null = null;
private lastError: string | null = null;

constructor(private readonly configService: ConfigService) {}

async onModuleInit() {
const url = this.configService.get<string>('REDIS_URL', 'redis://localhost:6379');
// Redis is optional. Without REDIS_URL there is nothing to connect to:
// falling back to localhost:6379 made ioredis retry forever on every
// default self-hosted install and log a warning every two seconds.
const url = this.configService.get<string>('REDIS_URL')?.trim();
if (!url) {
this.logger.log('REDIS_URL is not set: caching and rate-limit counters stay in memory.');
return;
}

this.client = new Redis(url, {
maxRetriesPerRequest: 3,
lazyConnect: true,
});

// Log a connection error once, not on every retry; say so when it recovers.
this.client.on('error', (err) => {
if (err.message === this.lastError) return;
this.lastError = err.message;
this.logger.warn(`Redis connection error: ${err.message}`);
});
this.client.on('ready', () => {
if (this.lastError) this.logger.log('Redis connection restored');
this.lastError = null;
});

try {
await this.client.connect();
this.logger.log('Redis connected');
} catch (err: any) {
this.logger.warn(`Redis not available: ${err.message}. Caching disabled.`);
this.logger.warn(`Redis not available: ${err.message}. Caching disabled until it is.`);
}
}

Expand All @@ -39,45 +55,50 @@ export class RedisService implements OnModuleInit, OnModuleDestroy {
}
}

/** REDIS_URL is set; the connection may still be down. */
get isConfigured(): boolean {
return this.client !== null;
}

get isConnected(): boolean {
return this.client?.status === 'ready';
}

async get(key: string): Promise<string | null> {
if (!this.isConnected) return null;
return this.client.get(key);
return this.client!.get(key);
}

async set(key: string, value: string, ttlSeconds?: number): Promise<void> {
if (!this.isConnected) return;
if (ttlSeconds) {
await this.client.set(key, value, 'EX', ttlSeconds);
await this.client!.set(key, value, 'EX', ttlSeconds);
} else {
await this.client.set(key, value);
await this.client!.set(key, value);
}
}

async del(key: string): Promise<void> {
if (!this.isConnected) return;
await this.client.del(key);
await this.client!.del(key);
}

async incr(key: string): Promise<number> {
if (!this.isConnected) return 0;
return this.client.incr(key);
return this.client!.incr(key);
}

async expire(key: string, ttlSeconds: number): Promise<void> {
if (!this.isConnected) return;
await this.client.expire(key, ttlSeconds);
await this.client!.expire(key, ttlSeconds);
}

async ttl(key: string): Promise<number> {
if (!this.isConnected) return -1;
return this.client.ttl(key);
return this.client!.ttl(key);
}

getClient(): Redis {
getClient(): Redis | null {
return this.client;
}
}
13 changes: 10 additions & 3 deletions packages/backend/src/health/health.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,15 @@ export class HealthController {
if (this.redis.isConnected) {
return { redis: { status: 'up' } };
}
// Redis is optional — report as up with a message so the health check
// does not fail when Redis is simply not configured.
return { redis: { status: 'up', message: 'Not configured (optional)' } };
// Redis is optional: report up with a message so the health check does
// not fail, but tell "not configured" apart from "configured and down".
return {
redis: {
status: 'up',
message: this.redis.isConfigured
? 'Configured but not reachable (caching disabled)'
: 'Not configured (optional)',
},
};
}
}
37 changes: 36 additions & 1 deletion packages/frontend/src/app/connectors/[id]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@
}

fetchConnector();
}, [token, id]);

Check warning on line 251 in packages/frontend/src/app/connectors/[id]/page.tsx

View workflow job for this annotation

GitHub Actions / Frontend (lint, typecheck, build)

React Hook useEffect has missing dependencies: 'fetchConnector' and 'searchParams'. Either include them or remove the dependency array

const resetOauth1Fields = () => {
setEditOauth1Key('');
Expand Down Expand Up @@ -1663,7 +1663,7 @@
{/* Show mapping summary */}
<div className="flex gap-3 mt-1.5 text-[10px] text-[var(--text-3)] flex-wrap">
{tool.endpointMapping?.path && (
<span className="font-mono break-all">{tool.endpointMapping.path}</span>
<ToolPathSummary path={tool.endpointMapping.path} />
)}
{tool.parameters?.properties && (() => {
const allParams = Object.keys(tool.parameters.properties);
Expand Down Expand Up @@ -1944,3 +1944,38 @@
if (!method || method === 'post') return 'client_secret_post';
return method;
}

/**
* The endpoint line on a tool card. For REST it is a short path; for database
* tools it is the whole SQL statement, which printed in full, wrapped mid-word
* at `break-all`, made one card as tall as the screen (the SAP HANA tools run
* to 40 lines). Long ones show their first line and open on request.
*/
function ToolPathSummary({ path }: { path: string }) {
const [open, setOpen] = useState(false);
const oneLine = path.replace(/\s+/g, ' ').trim();
if (oneLine.length <= 120 && !path.includes('\n')) {
return <span className="font-mono break-all">{path}</span>;
}
return (
<span className="flex min-w-0 max-w-full basis-full flex-col gap-1">
{open ? (
<pre className="max-h-80 overflow-auto whitespace-pre-wrap break-words rounded-[8px] bg-[var(--surface)] p-2 font-mono text-[11px] leading-relaxed text-[var(--text-2)]">
{path.trim()}
</pre>
) : (
<span className="truncate font-mono" title={oneLine}>
{oneLine}
</span>
)}
<button
type="button"
onClick={() => setOpen((v) => !v)}
className="self-start text-[var(--brand)] hover:underline"
aria-expanded={open}
>
{open ? 'Hide' : 'Show full'} {/^\s*(select|with)\b/i.test(path) ? 'SQL' : 'path'}
</button>
</span>
);
}
Loading
Loading