mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 02:49:33 +01:00
fix(discord): show the real reason a thread fails, keep the gateway across a save, refund the rate limit for a refused file
This commit is contained in:
1 parent
a698729fb6
commit
d044e49635
4 files changed
+54
-10
No files matched your search
+20
-8
@@ -286,6 +286,9 @@ function coerce(raw: unknown): DiscordConfig {
|
||||
};
|
||||
}
|
||||
|
||||
/** A failure with a reason worth showing as it is (unlike a network error, whose text is not). */
|
||||
class BridgeProblem extends Error {}
|
||||
|
||||
export async function readDiscordConfig(configDir: string): Promise<DiscordConfig> {
|
||||
try {
|
||||
return coerce(JSON.parse(await fs.readFile(discordConfigPath(configDir), 'utf-8')));
|
||||
@@ -674,6 +677,8 @@ export class DiscordBridge {
|
||||
private threads: Record<string, string> = {};
|
||||
private loaded = false;
|
||||
private gateway: DiscordGateway | null = null;
|
||||
/** The token the running gateway was started with ('' when none). */
|
||||
private gatewayToken = '';
|
||||
private lastSent = new Map<string, number>();
|
||||
private lastReply = new Map<string, number>();
|
||||
/** The last name seen per session, so only a real rename touches the thread. */
|
||||
@@ -762,10 +767,14 @@ export class DiscordBridge {
|
||||
this.threadsParent = parent;
|
||||
await this.persistThreads();
|
||||
}
|
||||
activeStartupHint = this.cfg.enabled && this.cfg.token && this.cfg.startupHint ? DISCORD_STARTUP_HINT : undefined;
|
||||
// A save that changed neither the switch nor the token keeps the gateway session, so it can resume.
|
||||
const want = this.cfg.enabled && this.cfg.token && !this.held ? this.cfg.token : '';
|
||||
if (this.gateway && want && want === this.gatewayToken) return;
|
||||
this.gateway?.stop();
|
||||
this.gateway = null;
|
||||
activeStartupHint = this.cfg.enabled && this.cfg.token && this.cfg.startupHint ? DISCORD_STARTUP_HINT : undefined;
|
||||
if (this.cfg.enabled && this.cfg.token && !this.held) {
|
||||
this.gatewayToken = want;
|
||||
if (want) {
|
||||
this.gateway = new DiscordGateway(
|
||||
() => this.cfg.token,
|
||||
(m) => void this.onGatewayMessage(m).catch(() => undefined),
|
||||
@@ -779,6 +788,7 @@ export class DiscordBridge {
|
||||
stop(): void {
|
||||
this.gateway?.stop();
|
||||
this.gateway = null;
|
||||
this.gatewayToken = '';
|
||||
activeStartupHint = undefined;
|
||||
this.lastSent.clear();
|
||||
this.lastReply.clear();
|
||||
@@ -799,9 +809,11 @@ export class DiscordBridge {
|
||||
? 'Channel not found (check the IDs)'
|
||||
: status
|
||||
? `HTTP ${status}`
|
||||
: (err as { name?: string })?.name === 'TimeoutError'
|
||||
? 'Timed out'
|
||||
: 'Network error';
|
||||
: err instanceof BridgeProblem
|
||||
? err.message
|
||||
: (err as { name?: string })?.name === 'TimeoutError'
|
||||
? 'Timed out'
|
||||
: 'Network error';
|
||||
return { ok: false, status, error, at: this.now() };
|
||||
}
|
||||
|
||||
@@ -823,12 +835,12 @@ export class DiscordBridge {
|
||||
const p = (mine.p = (async () => {
|
||||
try {
|
||||
const t = await this.api.createThread(parent, threadNameFor(name, sessionId));
|
||||
if (!t?.id) return this.fail(new Error('Discord did not return a thread'));
|
||||
if (!t?.id) return this.fail(new BridgeProblem('Discord did not return a thread'));
|
||||
// A save moved the bridge to another channel meanwhile: this thread belongs to the old one.
|
||||
if (parent !== this.cfg.parentChannelId) return this.fail(new Error('The parent channel changed'));
|
||||
if (parent !== this.cfg.parentChannelId) return this.fail(new BridgeProblem('The parent channel changed'));
|
||||
if (this.closedWhilePending.has(sessionId)) {
|
||||
await this.closeThread(t.id);
|
||||
return this.fail(new Error('The tab was closed'));
|
||||
return this.fail(new BridgeProblem('The tab was closed'));
|
||||
}
|
||||
this.threads[sessionId] = t.id;
|
||||
await this.persistThreads();
|
||||
|
||||
@@ -77,6 +77,12 @@ function consumeNotifyToken(sessionId: string, now: number = Date.now()): boolea
|
||||
return true;
|
||||
}
|
||||
|
||||
/** Give back a token spent on a request that was refused before anything was posted. */
|
||||
function refundNotifyToken(sessionId: string): void {
|
||||
const b = notifyBuckets.get(sessionId);
|
||||
if (b) b.tokens = Math.min(NOTIFY_PER_MINUTE, b.tokens + 1);
|
||||
}
|
||||
|
||||
/** Test hook: forget every bucket between runs. */
|
||||
export function _resetNotifyBuckets(): void {
|
||||
notifyBuckets.clear();
|
||||
@@ -197,6 +203,7 @@ export function registerDiscordRoutes(app: FastifyInstance, deps: DiscordRouteDe
|
||||
if (file !== undefined) {
|
||||
const problem = await fileProblem(session.workingDir, !!session.remote, file);
|
||||
if (typeof problem === 'string') {
|
||||
refundNotifyToken(session.id);
|
||||
reply.code(400);
|
||||
return createErrorResponse(ApiErrorCode.INVALID_INPUT, problem);
|
||||
}
|
||||
|
||||
@@ -582,7 +582,20 @@ describe('DiscordBridge', () => {
|
||||
it('names a thread that Discord answers without an id, instead of reporting an old result', async () => {
|
||||
const b = bridge({}, async (url) => (String(url).endsWith('/threads') ? jsonRes({}) : jsonRes({ id: 'msg' })));
|
||||
const r = await b.notify(MSG);
|
||||
expect(r).toMatchObject({ ok: false });
|
||||
expect(r).toMatchObject({ ok: false, error: 'Discord did not return a thread' });
|
||||
b.stop();
|
||||
});
|
||||
|
||||
it('keeps the gateway session across a save that changes neither the switch nor the token', async () => {
|
||||
const b = bridge();
|
||||
await b.reload();
|
||||
const first = (b as unknown as { gateway: unknown }).gateway;
|
||||
expect(first).not.toBeNull();
|
||||
await b.reload();
|
||||
expect((b as unknown as { gateway: unknown }).gateway).toBe(first);
|
||||
await writeDiscordConfig(dir, { ...CFG, token: 'another-token' });
|
||||
await b.reload();
|
||||
expect((b as unknown as { gateway: unknown }).gateway).not.toBe(first);
|
||||
b.stop();
|
||||
});
|
||||
|
||||
@@ -692,8 +705,9 @@ describe('DiscordBridge', () => {
|
||||
await vi.waitFor(() => expect(posts.some((p) => p.path.endsWith('/threads'))).toBe(true));
|
||||
await b.sessionDeleted('s1');
|
||||
release();
|
||||
await first;
|
||||
const out = await first;
|
||||
expect(posts).toContainEqual({ path: `/channels/${THREAD}`, method: 'PATCH', body: { archived: true } });
|
||||
expect(out).toMatchObject({ ok: false, error: 'The tab was closed' });
|
||||
expect(await readThreadMap(dir)).toEqual({});
|
||||
b.stop();
|
||||
});
|
||||
|
||||
@@ -216,6 +216,17 @@ describe('POST /api/sessions/:id/discord-notify', () => {
|
||||
bridge.stop();
|
||||
});
|
||||
|
||||
it('does not spend rate-limit tokens on a request refused for its file', async () => {
|
||||
const { app, bridge } = await harness();
|
||||
await app.inject({ method: 'PUT', url: '/api/discord', payload: FULL });
|
||||
for (let i = 0; i < 8; i++) {
|
||||
const bad = await app.inject({ method: 'POST', url, payload: { message: 'x', file: 'no-such-file.txt' } });
|
||||
expect(bad.statusCode).toBe(400);
|
||||
}
|
||||
expect((await app.inject({ method: 'POST', url, payload: { message: 'still allowed' } })).statusCode).toBe(200);
|
||||
bridge.stop();
|
||||
});
|
||||
|
||||
it('rejects an empty or oversized message', async () => {
|
||||
const { app } = await harness();
|
||||
const url = `/api/sessions/test-session-1/discord-notify`;
|
||||
|
||||
Reference in new issue
Block a user