Skip to content

Commit 7ecb9ac

Browse files
authored
feat: DH-19897: Handle missing jsapi/dh-internal.js files on core workers (#2501)
DH-19897: Handle missing jsapi/dh-internal.js files on core workers I added a flag to resolve 404s as empty content instead of throwing an error. 404s were already differentiated from other errors that get retry attempts, so it was a simple change. Testing I tested locally by renaming `jsapi/dh-internal.js` in the `loadModules` call to `jsapi/dh-internal2.js` to mimick a 404. Confirmed empty content file gets created in VS Code extension. I also tested stopping the server which produces a different error that invokes the retry mechanism instead of a 404. Worst case scenario, if we ever get out of sorts, restarting VS Code will delete the tmp download folder and retry everything again.
1 parent 8c8edd5 commit 7ecb9ac

File tree

3 files changed

+112
-44
lines changed

3 files changed

+112
-44
lines changed

packages/jsapi-nodejs/src/errorUtils.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
import { STATUS_CODES } from 'node:http';
2+
3+
/** General Http Error class. */
4+
export class HttpError extends Error {
5+
constructor(
6+
public statusCode: number,
7+
message?: string
8+
) {
9+
super(message ?? STATUS_CODES[statusCode]);
10+
this.name = 'HttpError';
11+
}
12+
}
13+
114
/**
215
* Return true if given error has a code:string prop. Optionally check if the
316
* code matches a given value.

packages/jsapi-nodejs/src/loaderUtils.ts

Lines changed: 97 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,36 @@ import type { dh as DhType } from '@deephaven/jsapi-types';
55
import { downloadFromURL, urlToDirectoryName } from './serverUtils.js';
66
import { polyfillWs } from './polyfillWs.js';
77
import { ensureDirectoriesExist, getDownloadPaths } from './fsUtils.js';
8+
import { HttpError } from './errorUtils.js';
89

910
type NonEmptyArray<T> = [T, ...T[]];
1011

12+
const DH_CORE_MODULE = 'jsapi/dh-core.js' as const;
13+
const DH_INTERNAL_MODULE = 'jsapi/dh-internal.js' as const;
14+
1115
/** Transform downloaded content */
1216
export type PostDownloadTransform = (
1317
serverPath: string,
1418
content: string
1519
) => string;
1620

21+
export type PostDownloadErrorTransform = (
22+
serverPath: string,
23+
error: unknown
24+
) => string;
25+
1726
export type LoadModuleOptions = {
1827
serverUrl: URL;
1928
serverPaths: NonEmptyArray<string>;
20-
download: boolean | PostDownloadTransform;
2129
storageDir: string;
2230
targetModuleType: 'esm' | 'cjs';
23-
};
31+
} & (
32+
| { download: false }
33+
| {
34+
download: true | PostDownloadTransform;
35+
errorTransform?: PostDownloadErrorTransform;
36+
}
37+
);
2438

2539
/**
2640
* Load a list of modules from a server.
@@ -32,38 +46,56 @@ export type LoadModuleOptions = {
3246
* the modules will be downloaded and stored. If set to a `PostDownloadTransform`
3347
* function, the downloaded content will be passed to the function and the result
3448
* saved to disk.
49+
* @param errorTransform Optional function to transform errors that occur during
50+
* the download process. If not provided, errors will be thrown.
3551
* @param storageDir The directory to store the downloaded modules.
3652
* @param targetModuleType The type of module to load. Can be either 'esm' or 'cjs'.
3753
* @returns The default export of the first module in `serverPaths`.
3854
*/
39-
export async function loadModules<TMainModule>({
40-
serverUrl,
41-
serverPaths,
42-
download,
43-
storageDir,
44-
targetModuleType,
45-
}: LoadModuleOptions): Promise<TMainModule> {
55+
export async function loadModules<TMainModule>(
56+
options: LoadModuleOptions
57+
): Promise<TMainModule> {
4658
polyfillWs();
4759

60+
const { serverUrl, serverPaths, storageDir, targetModuleType } = options;
61+
4862
const serverStorageDir = path.join(storageDir, urlToDirectoryName(serverUrl));
4963

50-
if (download !== false) {
64+
if (options.download !== false) {
5165
ensureDirectoriesExist([serverStorageDir]);
5266

67+
// Handle rejected Promise from download
68+
const handleRejected = (reason: unknown, i: number): string => {
69+
if (typeof options.errorTransform === 'function') {
70+
return options.errorTransform(serverPaths[i], reason);
71+
}
72+
73+
throw reason;
74+
};
75+
76+
// Handle resolved Promise from download
77+
const handleResolved = (value: string, i: number): string => {
78+
if (typeof options.download === 'function') {
79+
return options.download(serverPaths[i], value);
80+
}
81+
82+
return value;
83+
};
84+
5385
// Download from server
5486
const serverUrls = serverPaths.map(
5587
serverPath => new URL(serverPath, serverUrl)
5688
);
57-
let contents = await Promise.all(
89+
90+
const settledResults = await Promise.allSettled(
5891
serverUrls.map(url => downloadFromURL(url))
5992
);
6093

61-
// Post-download transform
62-
if (typeof download === 'function') {
63-
contents = contents.map((content, i) =>
64-
download(serverPaths[i], content)
65-
);
66-
}
94+
const contents: string[] = settledResults.map((result, i) =>
95+
result.status === 'rejected'
96+
? handleRejected(result.reason, i)
97+
: handleResolved(result.value, i)
98+
);
6799

68100
// Write to disk
69101
const downloadPaths = getDownloadPaths(serverStorageDir, serverPaths);
@@ -111,37 +143,60 @@ export async function loadDhModules({
111143
globalThis.window = globalThis;
112144
}
113145

146+
// If target module type is `cjs`, we need to transform the downloaded content
147+
// by replaing some ESM specific syntax with CJS syntax.
148+
const cjsDownloadTransform: PostDownloadTransform = (
149+
serverPath: string,
150+
content: string
151+
): string => {
152+
if (serverPath === DH_CORE_MODULE) {
153+
return content
154+
.replace(
155+
`import {dhinternal} from './dh-internal.js';`,
156+
`const {dhinternal} = require("./dh-internal.js");`
157+
)
158+
.replace(`export default dh;`, `module.exports = dh;`);
159+
}
160+
161+
if (serverPath === DH_INTERNAL_MODULE) {
162+
return content.replace(
163+
`export{__webpack_exports__dhinternal as dhinternal};`,
164+
`module.exports={dhinternal:__webpack_exports__dhinternal};`
165+
);
166+
}
167+
168+
return content;
169+
};
170+
171+
// `dh-internal.js` module is being removed from future versions of DH core,
172+
// but there's not a great way for this library to know whether it's present
173+
// or not. Treat 404s as empty content to make things compatible with both
174+
// configurations.
175+
const errorTransform: PostDownloadErrorTransform = (
176+
serverPath: string,
177+
error: unknown
178+
): string => {
179+
if (
180+
serverPath === DH_INTERNAL_MODULE &&
181+
error instanceof HttpError &&
182+
error.statusCode === 404
183+
) {
184+
return '';
185+
}
186+
187+
throw error;
188+
};
189+
114190
const coreModule = await loadModules<
115191
typeof DhType & { default?: typeof DhType }
116192
>({
117193
serverUrl,
118-
serverPaths: ['jsapi/dh-core.js', 'jsapi/dh-internal.js'],
194+
serverPaths: [DH_CORE_MODULE, DH_INTERNAL_MODULE],
119195
storageDir,
120196
targetModuleType,
121-
download:
122-
targetModuleType === 'esm'
123-
? // ESM does not need any transformation since the server modules are already ESM.
124-
true
125-
: // CJS needs a post-download transform to convert the ESM modules to CJS.
126-
(serverPath, content) => {
127-
if (serverPath === 'jsapi/dh-core.js') {
128-
return content
129-
.replace(
130-
`import {dhinternal} from './dh-internal.js';`,
131-
`const {dhinternal} = require("./dh-internal.js");`
132-
)
133-
.replace(`export default dh;`, `module.exports = dh;`);
134-
}
135-
136-
if (serverPath === 'jsapi/dh-internal.js') {
137-
return content.replace(
138-
`export{__webpack_exports__dhinternal as dhinternal};`,
139-
`module.exports={dhinternal:__webpack_exports__dhinternal};`
140-
);
141-
}
142-
143-
return content;
144-
},
197+
// Download the module and transform it if the target module type is `cjs`.
198+
download: targetModuleType === 'esm' ? true : cjsDownloadTransform,
199+
errorTransform,
145200
});
146201

147202
// ESM uses `default` export. CJS does not.

packages/jsapi-nodejs/src/serverUtils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as http from 'node:http';
22
import * as https from 'node:https';
3-
import { hasErrorCode, isAggregateError } from './errorUtils.js';
3+
import { hasErrorCode, HttpError, isAggregateError } from './errorUtils.js';
44

55
export const SERVER_STATUS_CHECK_TIMEOUT = 3000;
66

@@ -46,7 +46,7 @@ export async function downloadFromURL(
4646

4747
res.on('end', async () => {
4848
if (res.statusCode === 404) {
49-
reject(new Error(`File not found: "${url}"`));
49+
reject(new HttpError(404, `File not found: "${url}"`));
5050
return;
5151
}
5252

0 commit comments

Comments
 (0)