Skip to content

Commit dcaf785

Browse files
Antoine de Chevignéclaude
andcommitted
Add cleanup logic, tests, and fix error handling for custom L1 parents
- Add cleanup logic to delete orphaned custom L1 parent if config creation fails - Add tests for startCustomL1ParentSync job (10 test cases) - Fix inconsistent error handling (res -> next) in Orbit config endpoint - Add count method to OrbitChainConfig and OpChainConfig mocks Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent ca8e021 commit dcaf785

File tree

4 files changed

+184
-3
lines changed

4 files changed

+184
-3
lines changed

run/api/explorers.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,14 @@ router.post('/:id/orbitConfig', authMiddleware, async (req, res, next) => {
328328
try {
329329
config = await db.createOrbitConfig(userId, data.id, configParams);
330330
} catch(error) {
331+
// Clean up orphaned custom L1 parent if config creation fails
332+
if (hasNewCustomL1 && configParams.parentWorkspaceId) {
333+
try {
334+
await db.deleteCustomL1Parent(userId, configParams.parentWorkspaceId);
335+
} catch(cleanupError) {
336+
// Log but don't fail on cleanup error
337+
}
338+
}
331339
return managedError(error, req, res);
332340
}
333341

@@ -343,7 +351,7 @@ router.post('/:id/orbitConfig', authMiddleware, async (req, res, next) => {
343351

344352
res.status(200).json({ config });
345353
} catch (error) {
346-
unmanagedError(error, req, res);
354+
unmanagedError(error, req, next);
347355
}
348356
});
349357

@@ -499,6 +507,14 @@ router.post('/:id/opConfig', authMiddleware, async (req, res, next) => {
499507
try {
500508
config = await db.createOpConfig(userId, data.id, configParams);
501509
} catch(error) {
510+
// Clean up orphaned custom L1 parent if config creation fails
511+
if (hasNewCustomL1 && configParams.parentWorkspaceId) {
512+
try {
513+
await db.deleteCustomL1Parent(userId, configParams.parentWorkspaceId);
514+
} catch(cleanupError) {
515+
// Log but don't fail on cleanup error
516+
}
517+
}
502518
return managedError(error, req, res);
503519
}
504520

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* @fileoverview Tests for startCustomL1ParentSync job.
3+
*/
4+
require('../mocks/lib/queue');
5+
require('../mocks/lib/logger');
6+
require('../mocks/models');
7+
8+
const { Workspace, OrbitChainConfig, OpChainConfig } = require('../mocks/models');
9+
const startCustomL1ParentSync = require('../../jobs/startCustomL1ParentSync');
10+
11+
jest.mock('../../lib/queue');
12+
jest.mock('../../lib/logger');
13+
jest.mock('../../lib/pm2');
14+
jest.mock('../../lib/env');
15+
16+
const PM2 = require('../../lib/pm2');
17+
const { getPm2Host, getPm2Secret } = require('../../lib/env');
18+
19+
beforeEach(() => jest.clearAllMocks());
20+
21+
describe('startCustomL1ParentSync', () => {
22+
let mockWorkspace;
23+
let mockPm2Instance;
24+
25+
beforeEach(() => {
26+
jest.clearAllMocks();
27+
28+
getPm2Host.mockReturnValue('http://localhost:3000');
29+
getPm2Secret.mockReturnValue('test-secret');
30+
31+
mockWorkspace = {
32+
id: 113,
33+
name: 'My Custom L1',
34+
rpcServer: 'https://my-custom-l1.com/rpc',
35+
isCustomL1Parent: true
36+
};
37+
38+
mockPm2Instance = {
39+
find: jest.fn(),
40+
start: jest.fn()
41+
};
42+
43+
PM2.mockImplementation(() => mockPm2Instance);
44+
});
45+
46+
describe('when workspace is a valid custom L1 parent', () => {
47+
it('should start sync when workspace has OP children', async () => {
48+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
49+
OrbitChainConfig.count.mockResolvedValue(0);
50+
OpChainConfig.count.mockResolvedValue(1);
51+
mockPm2Instance.find.mockResolvedValue({ data: null });
52+
mockPm2Instance.start.mockResolvedValue({});
53+
54+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
55+
56+
expect(Workspace.findByPk).toHaveBeenCalledWith(113);
57+
expect(OpChainConfig.count).toHaveBeenCalledWith({ where: { parentWorkspaceId: 113 } });
58+
expect(mockPm2Instance.start).toHaveBeenCalledWith('custom-l1-113', 113);
59+
expect(result).toBe('Started sync for workspace 113');
60+
});
61+
62+
it('should start sync when workspace has Orbit children', async () => {
63+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
64+
OrbitChainConfig.count.mockResolvedValue(2);
65+
OpChainConfig.count.mockResolvedValue(0);
66+
mockPm2Instance.find.mockResolvedValue({ data: null });
67+
mockPm2Instance.start.mockResolvedValue({});
68+
69+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
70+
71+
expect(OrbitChainConfig.count).toHaveBeenCalledWith({ where: { parentWorkspaceId: 113 } });
72+
expect(mockPm2Instance.start).toHaveBeenCalledWith('custom-l1-113', 113);
73+
expect(result).toBe('Started sync for workspace 113');
74+
});
75+
76+
it('should skip if sync is already running', async () => {
77+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
78+
OrbitChainConfig.count.mockResolvedValue(1);
79+
OpChainConfig.count.mockResolvedValue(0);
80+
mockPm2Instance.find.mockResolvedValue({ data: { pm2_env: { status: 'online' } } });
81+
82+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
83+
84+
expect(mockPm2Instance.start).not.toHaveBeenCalled();
85+
expect(result).toBe('Sync already running for workspace 113');
86+
});
87+
});
88+
89+
describe('when workspace should not be synced', () => {
90+
it('should skip if workspace not found', async () => {
91+
Workspace.findByPk.mockResolvedValue(null);
92+
93+
const result = await startCustomL1ParentSync({ data: { workspaceId: 999 } });
94+
95+
expect(result).toBe('Workspace 999 not found');
96+
expect(mockPm2Instance.start).not.toHaveBeenCalled();
97+
});
98+
99+
it('should skip if workspace is not a custom L1 parent', async () => {
100+
Workspace.findByPk.mockResolvedValue({ ...mockWorkspace, isCustomL1Parent: false });
101+
102+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
103+
104+
expect(result).toBe('Workspace 113 is not a custom L1 parent');
105+
expect(mockPm2Instance.start).not.toHaveBeenCalled();
106+
});
107+
108+
it('should skip if workspace has no L2 children', async () => {
109+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
110+
OrbitChainConfig.count.mockResolvedValue(0);
111+
OpChainConfig.count.mockResolvedValue(0);
112+
113+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
114+
115+
expect(result).toBe('No L2 children for workspace 113');
116+
expect(mockPm2Instance.start).not.toHaveBeenCalled();
117+
});
118+
});
119+
120+
describe('when PM2 is not configured', () => {
121+
it('should skip if PM2 host is not set', async () => {
122+
getPm2Host.mockReturnValue(null);
123+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
124+
OrbitChainConfig.count.mockResolvedValue(1);
125+
OpChainConfig.count.mockResolvedValue(0);
126+
127+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
128+
129+
expect(result).toBe('PM2 not configured');
130+
expect(mockPm2Instance.start).not.toHaveBeenCalled();
131+
});
132+
133+
it('should skip if PM2 secret is not set', async () => {
134+
getPm2Secret.mockReturnValue(null);
135+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
136+
OrbitChainConfig.count.mockResolvedValue(1);
137+
OpChainConfig.count.mockResolvedValue(0);
138+
139+
const result = await startCustomL1ParentSync({ data: { workspaceId: 113 } });
140+
141+
expect(result).toBe('PM2 not configured');
142+
expect(mockPm2Instance.start).not.toHaveBeenCalled();
143+
});
144+
});
145+
146+
describe('error handling', () => {
147+
it('should throw if workspaceId is missing', async () => {
148+
await expect(startCustomL1ParentSync({ data: {} }))
149+
.rejects.toThrow('Missing workspaceId parameter');
150+
});
151+
152+
it('should throw if PM2 start fails', async () => {
153+
Workspace.findByPk.mockResolvedValue(mockWorkspace);
154+
OrbitChainConfig.count.mockResolvedValue(1);
155+
OpChainConfig.count.mockResolvedValue(0);
156+
mockPm2Instance.find.mockResolvedValue({ data: null });
157+
mockPm2Instance.start.mockRejectedValue(new Error('PM2 connection failed'));
158+
159+
await expect(startCustomL1ParentSync({ data: { workspaceId: 113 } }))
160+
.rejects.toThrow('PM2 connection failed');
161+
});
162+
});
163+
});

run/tests/mocks/models/OpChainConfig.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ const OpChainConfig = {
33
create: jest.fn(),
44
update: jest.fn(),
55
findAll: jest.fn(),
6-
destroy: jest.fn()
6+
destroy: jest.fn(),
7+
count: jest.fn()
78
};
89

910
module.exports = { OpChainConfig };

run/tests/mocks/models/OrbitChainConfig.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ const OrbitChainConfig = {
44
create: jest.fn(),
55
update: jest.fn(),
66
findAll: jest.fn(),
7-
destroy: jest.fn()
7+
destroy: jest.fn(),
8+
count: jest.fn()
89
};
910

1011
module.exports = { OrbitChainConfig };

0 commit comments

Comments
 (0)