Skip to content

Commit fc8b101

Browse files
authored
Initialize workers from endpoint rather than blob (#10860)
* Refactor Monaco worker loading to use served URLs instead of inline source Changes worker initialization from blob-based inline source to URL-based loading with cache-busting support. Workers are now served as static assets from the server with build hash-based URLs. Key changes: - Remove worker_store system for inline worker registration - Remove raw-loader dependency and usage - Add setBuildHash() to configure worker URL generation - Update CSP Report-Only to allow blob: workers during transition - Centralize worker creation in monaco_environment - Update PPL and xjson language modules to use new system Signed-off-by: Daniel Rowe <[email protected]> * add getWorker mock Signed-off-by: Daniel Rowe <[email protected]> * Revert yarn.lock reorganization changes Signed-off-by: Daniel Rowe <[email protected]> --------- Signed-off-by: Daniel Rowe <[email protected]> Signed-off-by: daniel <[email protected]>
1 parent 12f0b23 commit fc8b101

File tree

16 files changed

+346
-75
lines changed

16 files changed

+346
-75
lines changed

packages/osd-monaco/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
"css-loader": "^5.2.7",
2323
"del": "^6.1.1",
2424
"file-loader": "^6.2.0",
25-
"raw-loader": "^4.0.2",
2625
"style-loader": "^1.1.3",
2726
"supports-color": "^7.0.0",
2827
"typescript": "4.5.5",

packages/osd-monaco/src/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@
3131
export { monaco } from './monaco';
3232
export { XJsonLang } from './xjson';
3333
export { PPLLang } from './ppl';
34-
import './json';
3534

3635
/* eslint-disable-next-line @osd/eslint/module_migration */
3736
import * as BarePluginApi from 'monaco-editor/esm/vs/editor/editor.api';
3837
export { BarePluginApi };
3938
import './monaco_environment';
40-
export * from './worker_store';
39+
export { setBuildHash } from './monaco_environment';
4140
export { WORKER_FILES, getWorkerUrl, getWorkerUrls, WorkerId } from './worker_config';

packages/osd-monaco/src/json/index.ts

Lines changed: 0 additions & 10 deletions
This file was deleted.
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import { WorkerLabels } from './worker_config';
7+
import { setBuildHash, getWorker } from './monaco_environment';
8+
9+
// Mock Worker constructor
10+
class MockWorker {
11+
constructor(public url: string) {}
12+
postMessage = jest.fn();
13+
terminate = jest.fn();
14+
onmessage: ((event: MessageEvent) => void) | null = null;
15+
onerror: ((event: ErrorEvent) => void) | null = null;
16+
}
17+
18+
global.Worker = MockWorker as any;
19+
20+
describe('monaco_environment', () => {
21+
describe('setBuildHash and getWorker', () => {
22+
it('should throw error if getWorker called before setBuildHash', () => {
23+
expect(() => getWorker(WorkerLabels.PPL)).toThrow('Build hash must be set');
24+
});
25+
26+
it('should create Worker after setBuildHash is called', () => {
27+
setBuildHash(12345);
28+
const worker = getWorker(WorkerLabels.PPL);
29+
30+
expect(worker).toBeInstanceOf(MockWorker);
31+
expect((worker as any).url).toBe('/12345/editor/workers/ppl.editor.worker.js');
32+
});
33+
34+
it('should use updated build hash on subsequent calls', () => {
35+
setBuildHash(111);
36+
setBuildHash(222);
37+
const worker = getWorker(WorkerLabels.PPL);
38+
39+
expect((worker as any).url).toBe('/222/editor/workers/ppl.editor.worker.js');
40+
});
41+
42+
it('should create new Worker instances on each call', () => {
43+
setBuildHash(12345);
44+
const worker1 = getWorker(WorkerLabels.PPL);
45+
const worker2 = getWorker(WorkerLabels.PPL);
46+
47+
expect(worker1).not.toBe(worker2);
48+
});
49+
50+
it('should work with all worker labels', () => {
51+
setBuildHash(12345);
52+
53+
const pplWorker = getWorker(WorkerLabels.PPL);
54+
const jsonWorker = getWorker(WorkerLabels.JSON);
55+
const xjsonWorker = getWorker(WorkerLabels.XJSON);
56+
57+
expect((pplWorker as any).url).toContain('ppl.editor.worker.js');
58+
expect((jsonWorker as any).url).toContain('json.editor.worker.js');
59+
expect((xjsonWorker as any).url).toContain('xjson.editor.worker.js');
60+
});
61+
62+
it('should throw error for invalid worker label', () => {
63+
setBuildHash(12345);
64+
expect(() => getWorker('invalid' as WorkerLabels)).toThrow('No worker available');
65+
});
66+
});
67+
});

packages/osd-monaco/src/monaco_environment.ts

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,38 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import { getWorker } from './worker_store';
6+
import { getWorkerUrl, WorkerLabels } from './worker_config';
7+
8+
let buildHash: number | undefined;
9+
10+
/**
11+
* Sets the build hash for worker URL generation.
12+
* This should be called during application initialization before any workers are created.
13+
* @param hash The build hash string
14+
*/
15+
export function setBuildHash(hash: number): void {
16+
buildHash = hash;
17+
}
18+
19+
/**
20+
* Creates a web worker from a served URL based on the worker label.
21+
* @param label The worker label/ID (e.g., 'ppl', 'json', 'xjson')
22+
* @returns A new Worker instance
23+
*/
24+
export function getWorker(label: WorkerLabels): Worker {
25+
if (!buildHash) {
26+
throw new Error('Build hash must be set before initializing editor workers');
27+
}
28+
29+
const workerUrl = getWorkerUrl(label, buildHash);
30+
if (!workerUrl) {
31+
throw new Error(`No worker available for language: ${label}`);
32+
}
33+
34+
return new Worker(workerUrl);
35+
}
736

837
// @ts-ignore
938
window.MonacoEnvironment = {
10-
getWorker: (_: string, label: string) => {
11-
const workerSrc = getWorker(label);
12-
if (workerSrc) {
13-
const blob = new Blob([workerSrc], { type: 'application/javascript' });
14-
const worker = new Worker(URL.createObjectURL(blob));
15-
return worker;
16-
}
17-
throw new Error(`No worker available for language: ${label}`);
18-
},
39+
getWorker,
1940
};

packages/osd-monaco/src/ppl/language.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,14 @@
55

66
import { monaco } from '../monaco';
77
import { ID, PPL_TOKEN_SETS } from './constants';
8-
import { registerWorker } from '../worker_store';
98
import { PPLWorkerProxyService } from './worker_proxy_service';
109
import { getPPLLanguageAnalyzer, PPLValidationResult } from './ppl_language_analyzer';
1110
import { getPPLDocumentationLink } from './ppl_documentation';
1211
import { pplOnTypeFormatProvider, pplRangeFormatProvider } from './formatter';
13-
// @ts-ignore
14-
import workerSrc from '!!raw-loader!../../target/public/ppl.editor.worker.js';
1512

1613
const PPL_LANGUAGE_ID = ID;
1714
const OWNER = 'PPL_WORKER';
1815

19-
// Register ppl worker to the worker map first
20-
registerWorker(ID, workerSrc);
21-
2216
// PPL worker proxy service for worker-based syntax highlighting
2317
const pplWorkerProxyService = new PPLWorkerProxyService();
2418

@@ -133,7 +127,7 @@ const processSyntaxHighlighting = async (model: monaco.editor.IModel) => {
133127
const content = model.getValue();
134128

135129
// Ensure worker is set up before validation - always call setup as it has internal check
136-
pplWorkerProxyService.setup(workerSrc);
130+
pplWorkerProxyService.setup();
137131

138132
// Get validation result from worker with timeout protection
139133
const validationResult = (await pplWorkerProxyService.validate(content)) as PPLValidationResult;
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import { PPLWorkerProxyService } from './worker_proxy_service';
7+
import * as monacoEnvironment from '../monaco_environment';
8+
import { WorkerLabels } from '../worker_config';
9+
10+
jest.mock('../monaco_environment');
11+
12+
describe('PPLWorkerProxyService', () => {
13+
let mockWorker: any;
14+
let service: PPLWorkerProxyService;
15+
16+
beforeEach(() => {
17+
mockWorker = {
18+
postMessage: jest.fn(),
19+
terminate: jest.fn(),
20+
onmessage: null,
21+
onerror: null,
22+
};
23+
24+
(monacoEnvironment.getWorker as jest.Mock).mockReturnValue(mockWorker);
25+
service = new PPLWorkerProxyService();
26+
});
27+
28+
afterEach(() => {
29+
jest.clearAllMocks();
30+
});
31+
32+
describe('setup', () => {
33+
it('should initialize worker for subsequent operations', async () => {
34+
service.setup();
35+
36+
expect(monacoEnvironment.getWorker).toHaveBeenCalledWith(WorkerLabels.PPL);
37+
38+
// Should be able to call tokenize/validate after setup
39+
const tokenizePromise = service.tokenize('test');
40+
expect(mockWorker.postMessage).toHaveBeenCalled();
41+
42+
// Simulate worker response
43+
const messageData = mockWorker.postMessage.mock.calls[0][0];
44+
mockWorker.onmessage({ data: { id: messageData.id, result: [] } });
45+
46+
await expect(tokenizePromise).resolves.toEqual([]);
47+
});
48+
49+
it('should be idempotent - multiple calls do not create multiple workers', () => {
50+
service.setup();
51+
service.setup();
52+
service.setup();
53+
54+
expect(monacoEnvironment.getWorker).toHaveBeenCalledTimes(1);
55+
});
56+
57+
it('should propagate error if setBuildHash not called', () => {
58+
(monacoEnvironment.getWorker as jest.Mock).mockImplementation(() => {
59+
throw new Error('Build hash must be set');
60+
});
61+
62+
expect(() => service.setup()).toThrow('Build hash must be set');
63+
});
64+
});
65+
66+
describe('tokenize', () => {
67+
it('should throw error if setup not called', async () => {
68+
await expect(service.tokenize('test')).rejects.toThrow('has not been setup');
69+
});
70+
71+
it('should return token array after setup', async () => {
72+
service.setup();
73+
74+
const tokenizePromise = service.tokenize('search source=logs');
75+
76+
// Simulate worker response
77+
const messageData = mockWorker.postMessage.mock.calls[0][0];
78+
const mockTokens = [{ type: 'search', value: 'search' }];
79+
mockWorker.onmessage({ data: { id: messageData.id, result: mockTokens } });
80+
81+
const result = await tokenizePromise;
82+
expect(result).toEqual(mockTokens);
83+
});
84+
});
85+
86+
describe('validate', () => {
87+
it('should throw error if setup not called', async () => {
88+
await expect(service.validate('test')).rejects.toThrow('has not been setup');
89+
});
90+
91+
it('should return validation result after setup', async () => {
92+
service.setup();
93+
94+
const validatePromise = service.validate('search source=logs');
95+
96+
// Simulate worker response
97+
const messageData = mockWorker.postMessage.mock.calls[0][0];
98+
const mockValidation = { isValid: true, errors: [] };
99+
mockWorker.onmessage({ data: { id: messageData.id, result: mockValidation } });
100+
101+
const result = await validatePromise;
102+
expect(result).toEqual(mockValidation);
103+
});
104+
105+
it('should handle error from worker', async () => {
106+
service.setup();
107+
108+
const validatePromise = service.validate('invalid');
109+
110+
// Simulate worker error response
111+
const messageData = mockWorker.postMessage.mock.calls[0][0];
112+
mockWorker.onmessage({ data: { id: messageData.id, error: 'Validation failed' } });
113+
114+
await expect(validatePromise).rejects.toThrow('Validation failed');
115+
});
116+
117+
it('should timeout if worker never responds', async () => {
118+
jest.useFakeTimers();
119+
service.setup();
120+
121+
const validatePromise = service.validate('test');
122+
123+
// Fast-forward time past the 5 second timeout
124+
jest.advanceTimersByTime(5001);
125+
126+
await expect(validatePromise).rejects.toThrow('Worker timeout');
127+
128+
jest.useRealTimers();
129+
});
130+
});
131+
132+
describe('stop', () => {
133+
it('should clean up worker resources', async () => {
134+
service.setup();
135+
service.stop();
136+
137+
expect(mockWorker.terminate).toHaveBeenCalled();
138+
139+
// Subsequent operations should fail
140+
await expect(service.tokenize('test')).rejects.toThrow('has not been setup');
141+
});
142+
143+
it('should be safe to call without setup', () => {
144+
expect(() => service.stop()).not.toThrow();
145+
});
146+
147+
it('should be safe to call multiple times', () => {
148+
service.setup();
149+
service.stop();
150+
service.stop();
151+
152+
expect(mockWorker.terminate).toHaveBeenCalledTimes(1);
153+
});
154+
});
155+
});

packages/osd-monaco/src/ppl/worker_proxy_service.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
*/
55

66
import { PPLValidationResult, PPLToken } from './ppl_language_analyzer';
7-
import { ID } from './constants';
7+
import { getWorker } from '../monaco_environment';
8+
import { WorkerLabels } from '../worker_config';
89

910
/**
1011
* Simple worker proxy that communicates with PPL worker
@@ -17,28 +18,30 @@ export class PPLWorkerProxyService {
1718
/**
1819
* Set up the worker
1920
*/
20-
public setup(workerSrc: string) {
21+
public setup() {
2122
if (this.worker) {
2223
return; // Already set up
2324
}
2425

25-
// Create worker from source
26-
const blob = new Blob([workerSrc], { type: 'application/javascript' });
27-
this.worker = new Worker(URL.createObjectURL(blob));
26+
// Create worker from served URL
27+
this.worker = getWorker(WorkerLabels.PPL);
2828

2929
// Handle messages from worker
3030
this.worker.onmessage = (e: MessageEvent) => {
3131
const { id, result, error } = e.data;
3232
const pending = this.pendingMessages.get(id);
3333

34-
if (pending) {
35-
this.pendingMessages.delete(id);
36-
if (error) {
37-
pending.reject(new Error(error));
38-
} else {
39-
pending.resolve(result);
40-
}
34+
if (!pending) {
35+
return;
36+
}
37+
38+
this.pendingMessages.delete(id);
39+
if (error) {
40+
pending.reject(new Error(error));
41+
return;
4142
}
43+
44+
pending.resolve(result);
4245
};
4346

4447
// Handle worker errors

0 commit comments

Comments
 (0)