From 501ea4a227369090f1674fb971e4ffee5c098244 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 07:18:39 +0000 Subject: [PATCH] refactor(sdk): remove dead code and de-duplicate SDK/CLI helpers Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../src/commands/template/generators/types.ts | 21 +++ packages/cli/src/commands/template/init.ts | 20 +-- packages/cli/src/commands/template/migrate.ts | 20 +-- packages/cli/src/utils/format.ts | 4 - packages/js-sdk/src/sandbox/index.ts | 129 +++++++---------- packages/js-sdk/src/secret.ts | 52 +++---- packages/js-sdk/src/volume/client.ts | 5 +- packages/js-sdk/src/volume/index.ts | 134 ++++++------------ packages/js-sdk/tests/sandbox/urls.test.ts | 23 +++ packages/python-sdk/e2b/sandbox/main.py | 81 +++++------ 10 files changed, 203 insertions(+), 286 deletions(-) diff --git a/packages/cli/src/commands/template/generators/types.ts b/packages/cli/src/commands/template/generators/types.ts index cd1bff4790..5b7fd1d86a 100644 --- a/packages/cli/src/commands/template/generators/types.ts +++ b/packages/cli/src/commands/template/generators/types.ts @@ -10,6 +10,27 @@ export const languageDisplay = { [Language.PythonAsync]: 'Python (async)', } +/** + * Choices for the interactive target language prompt. + */ +export const languageChoices = [ + { + name: languageDisplay[Language.TypeScript], + value: Language.TypeScript, + description: 'Generate .ts files for JavaScript/TypeScript projects', + }, + { + name: languageDisplay[Language.PythonSync], + value: Language.PythonSync, + description: 'Generate synchronous Python template files', + }, + { + name: languageDisplay[Language.PythonAsync], + value: Language.PythonAsync, + description: 'Generate asynchronous Python template files', + }, +] + export interface TemplateJSON { fromImage?: string fromTemplate?: string diff --git a/packages/cli/src/commands/template/init.ts b/packages/cli/src/commands/template/init.ts index e1b83e7e61..15bb3b7218 100644 --- a/packages/cli/src/commands/template/init.ts +++ b/packages/cli/src/commands/template/init.ts @@ -12,6 +12,7 @@ import { generateAndWriteTemplateFiles, GeneratedFiles, Language, + languageChoices, languageDisplay, } from './generators' import { generateReadmeContent } from './generators/handlebars' @@ -202,24 +203,7 @@ export const initCommand = new commander.Command('init') } else { language = await select({ message: 'Select target language for template files:', - choices: [ - { - name: languageDisplay[Language.TypeScript], - value: Language.TypeScript, - description: - 'Generate .ts files for JavaScript/TypeScript projects', - }, - { - name: languageDisplay[Language.PythonSync], - value: Language.PythonSync, - description: 'Generate synchronous Python template files', - }, - { - name: languageDisplay[Language.PythonAsync], - value: Language.PythonAsync, - description: 'Generate asynchronous Python template files', - }, - ], + choices: languageChoices, default: Language.TypeScript, }) } diff --git a/packages/cli/src/commands/template/migrate.ts b/packages/cli/src/commands/template/migrate.ts index 6bfbb6d069..23ec0e7125 100644 --- a/packages/cli/src/commands/template/migrate.ts +++ b/packages/cli/src/commands/template/migrate.ts @@ -13,6 +13,7 @@ import { validateTemplateName } from '../../utils/templateName' import { generateAndWriteTemplateFiles, Language, + languageChoices, languageDisplay, } from './generators' @@ -211,24 +212,7 @@ export const migrateCommand = new commander.Command('migrate') // Prompt for language selection language = await select({ message: 'Select target language for Template SDK:', - choices: [ - { - name: languageDisplay[Language.TypeScript], - value: Language.TypeScript, - description: - 'Generate .ts files for JavaScript/TypeScript projects', - }, - { - name: languageDisplay[Language.PythonSync], - value: Language.PythonSync, - description: 'Generate synchronous Python template files', - }, - { - name: languageDisplay[Language.PythonAsync], - value: Language.PythonAsync, - description: 'Generate asynchronous Python template files', - }, - ], + choices: languageChoices, default: Language.TypeScript, }) } diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 900f10a9dc..3f6792c95f 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -81,10 +81,6 @@ export function asLocalRelative(absolutePathInLocal?: string) { return asLocal('./' + cwdRelative(absolutePathInLocal)) } -export function asBuildLogs(content: string) { - return chalk.default.blueBright(content) -} - export function withUnderline(content: string) { return chalk.default.underline(content) } diff --git a/packages/js-sdk/src/sandbox/index.ts b/packages/js-sdk/src/sandbox/index.ts index f22de94e07..bd301b7dd3 100644 --- a/packages/js-sdk/src/sandbox/index.ts +++ b/packages/js-sdk/src/sandbox/index.ts @@ -750,46 +750,7 @@ export class Sandbox extends SandboxApi { * @returns URL for uploading file. */ async uploadUrl(path?: string, opts?: SandboxUrlOpts) { - opts = opts ?? {} - - const useSignature = !!this.envdAccessToken - - if (!useSignature && opts.useSignatureExpiration != undefined) { - throw new InvalidArgumentError( - 'Signature expiration can be used only when sandbox is created as secured.' - ) - } - - let username = opts.user - if ( - username == undefined && - compareVersions(this.envdApi.version, ENVD_DEFAULT_USER) < 0 - ) { - username = defaultUsername - } - - const filePath = path ?? '' - const fileUrl = this.fileUrl(filePath, username) - - if (useSignature) { - const url = new URL(fileUrl) - const sig = await getSignature({ - path: filePath, - operation: 'write', - user: username, - expirationInSeconds: opts.useSignatureExpiration, - envdAccessToken: this.envdAccessToken, - }) - - url.searchParams.set('signature', sig.signature) - if (sig.expiration) { - url.searchParams.set('signature_expiration', sig.expiration.toString()) - } - - return url.toString() - } - - return fileUrl + return await this.fileOperationUrl(path ?? '', 'write', opts) } /** @@ -802,45 +763,7 @@ export class Sandbox extends SandboxApi { * @returns URL for downloading file. */ async downloadUrl(path: string, opts?: SandboxUrlOpts) { - opts = opts ?? {} - - const useSignature = !!this.envdAccessToken - - if (!useSignature && opts.useSignatureExpiration != undefined) { - throw new InvalidArgumentError( - 'Signature expiration can be used only when sandbox is created as secured.' - ) - } - - let username = opts.user - if ( - username == undefined && - compareVersions(this.envdApi.version, ENVD_DEFAULT_USER) < 0 - ) { - username = defaultUsername - } - - const fileUrl = this.fileUrl(path, username) - - if (useSignature) { - const url = new URL(fileUrl) - const sig = await getSignature({ - path, - operation: 'read', - user: username, - expirationInSeconds: opts.useSignatureExpiration, - envdAccessToken: this.envdAccessToken, - }) - - url.searchParams.set('signature', sig.signature) - if (sig.expiration) { - url.searchParams.set('signature_expiration', sig.expiration.toString()) - } - - return url.toString() - } - - return fileUrl + return await this.fileOperationUrl(path, 'read', opts) } /** @@ -896,6 +819,54 @@ export class Sandbox extends SandboxApi { ) as ConnectionOpts & T } + /** + * Build the `/files` URL for a read or write operation, signing it when the + * sandbox is secured. + */ + private async fileOperationUrl( + path: string, + operation: 'read' | 'write', + opts?: SandboxUrlOpts + ): Promise { + const useSignature = !!this.envdAccessToken + + if (!useSignature && opts?.useSignatureExpiration != undefined) { + throw new InvalidArgumentError( + 'Signature expiration can be used only when sandbox is created as secured.' + ) + } + + let username = opts?.user + if ( + username == undefined && + compareVersions(this.envdApi.version, ENVD_DEFAULT_USER) < 0 + ) { + username = defaultUsername + } + + const fileUrl = this.fileUrl(path, username) + + if (!useSignature) { + return fileUrl + } + + const url = new URL(fileUrl) + const sig = await getSignature({ + path, + operation, + user: username, + expirationInSeconds: opts?.useSignatureExpiration, + envdAccessToken: this.envdAccessToken, + }) + + url.searchParams.set('signature', sig.signature) + if (sig.expiration) { + url.searchParams.set('signature_expiration', sig.expiration.toString()) + } + + return url.toString() + } + private fileUrl(path: string | undefined, username: string | undefined) { const url = new URL('/files', this.envdDirectUrl) diff --git a/packages/js-sdk/src/secret.ts b/packages/js-sdk/src/secret.ts index 37ae24bea3..d197e253b5 100644 --- a/packages/js-sdk/src/secret.ts +++ b/packages/js-sdk/src/secret.ts @@ -1,3 +1,5 @@ +import type { FetchResponse } from 'openapi-fetch' + import { ApiClient, components, handleApiError } from './api' import { ClientFactory, @@ -100,6 +102,23 @@ function convertSecretInfo( } } +/** + * Convert a response returning a single secret into {@link SecretInfo}, + * throwing for error responses and empty bodies. + */ +function secretInfoFromResponse(res: FetchResponse): SecretInfo { + const err = handleApiError(res, SecretError) + if (err) { + throw err + } + + if (!res.data) { + throw new Error('Response data is missing') + } + + return convertSecretInfo(res.data) +} + /** * Paginator for listing secrets. * @@ -182,16 +201,7 @@ export class Secret extends ClientFactory { signal: config.getSignal(apiOpts?.requestTimeoutMs, apiOpts?.signal), }) - const err = handleApiError(res, SecretError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertSecretInfo(res.data) + return secretInfoFromResponse(res) } /** @@ -230,16 +240,7 @@ export class Secret extends ClientFactory { throw new SecretNotFoundError(`Secret ${secret} not found`) } - const err = handleApiError(res, SecretError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertSecretInfo(res.data) + return secretInfoFromResponse(res) } /** @@ -272,16 +273,7 @@ export class Secret extends ClientFactory { throw new SecretNotFoundError(`Secret ${secret} not found`) } - const err = handleApiError(res, SecretError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertSecretInfo(res.data) + return secretInfoFromResponse(res) } /** diff --git a/packages/js-sdk/src/volume/client.ts b/packages/js-sdk/src/volume/client.ts index d63057b597..d9c005b133 100644 --- a/packages/js-sdk/src/volume/client.ts +++ b/packages/js-sdk/src/volume/client.ts @@ -3,11 +3,10 @@ import createClient from 'openapi-fetch' import type { components, paths } from './schema.gen' import { defaultHeaders, getEnvVar } from '../api/metadata' import { createApiFetch } from '../api/http2' -import { buildRequestSignal } from '../connectionConfig' +import { buildRequestSignal, REQUEST_TIMEOUT_MS } from '../connectionConfig' import { createApiLogger, Logger } from '../logs' import type { Volume } from './index' -const REQUEST_TIMEOUT_MS = 60_000 // 60 seconds const FILE_TIMEOUT_MS = 3_600_000 // 1 hour export interface VolumeApiOpts { @@ -135,5 +134,5 @@ class VolumeApiClient { } } -export type { components as VolumeApiComponents, paths as VolumeApiPaths } +export type { components as VolumeApiComponents } export { VolumeApiClient, FILE_TIMEOUT_MS } diff --git a/packages/js-sdk/src/volume/index.ts b/packages/js-sdk/src/volume/index.ts index 4563d0f9d8..d1dd90bc25 100644 --- a/packages/js-sdk/src/volume/index.ts +++ b/packages/js-sdk/src/volume/index.ts @@ -1,3 +1,5 @@ +import type { FetchResponse } from 'openapi-fetch' + import { ApiClient, handleApiError, components as ApiComponents } from '../api' import { VolumeApiClient, @@ -45,6 +47,43 @@ function convertVolumeEntryStat( } } +/** + * Throw for any non-2xx volume-content response, mapping a 404 to the path + * not existing. + */ +function throwOnVolumePathError( + res: FetchResponse, + path: string +): void { + if (res.response.status === 404) { + throw new VolumePathNotFoundError(`Path ${path} not found`) + } + + const err = handleApiError(res, VolumeError) + if (err) { + throw err + } +} + +/** + * Convert a volume-content response that returns a single entry into an SDK + * {@link VolumeEntryStat}, throwing for error responses and empty bodies. + */ +function volumeEntryStatFromResponse( + res: FetchResponse, + path: string +): VolumeEntryStat { + throwOnVolumePathError(res, path) + + if (!res.data) { + throw new Error('Response data is missing') + } + + return convertVolumeEntryStat( + res.data as VolumeApiComponents['schemas']['VolumeEntryStat'] + ) +} + /** * Module for interacting with E2B volumes. * @@ -311,14 +350,7 @@ export class Volume extends ClientFactory { signal: config.getSignal(), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } + throwOnVolumePathError(res, path) // VolumeDirectoryListing is an array according to the spec const entries = Array.isArray(res.data) ? res.data : [] @@ -355,22 +387,7 @@ export class Volume extends ClientFactory { signal: config.getSignal(), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertVolumeEntryStat( - res.data as VolumeApiComponents['schemas']['VolumeEntryStat'] - ) + return volumeEntryStatFromResponse(res, path) } /** @@ -397,22 +414,7 @@ export class Volume extends ClientFactory { signal: config.getSignal(), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertVolumeEntryStat( - res.data as VolumeApiComponents['schemas']['VolumeEntryStat'] - ) + return volumeEntryStatFromResponse(res, path) } /** @@ -472,22 +474,7 @@ export class Volume extends ClientFactory { signal: config.getSignal(), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertVolumeEntryStat( - res.data as VolumeApiComponents['schemas']['VolumeEntryStat'] - ) + return volumeEntryStatFromResponse(res, path) } /** @@ -630,14 +617,7 @@ export class Volume extends ClientFactory { signal: config.getSignal(), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } + throwOnVolumePathError(res, path) // When the file is empty, `res.data` is `undefined`, so empty values are synthesized below. if (format === 'bytes') { @@ -712,22 +692,7 @@ export class Volume extends ClientFactory { ...(streamed && { duplex: 'half' as const }), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } - - if (!res.data) { - throw new Error('Response data is missing') - } - - return convertVolumeEntryStat( - res.data as VolumeApiComponents['schemas']['VolumeEntryStat'] - ) + return volumeEntryStatFromResponse(res, path) } /** @@ -752,14 +717,7 @@ export class Volume extends ClientFactory { signal: config.getSignal(), }) - if (res.response.status === 404) { - throw new VolumePathNotFoundError(`Path ${path} not found`) - } - - const err = handleApiError(res, VolumeError) - if (err) { - throw err - } + throwOnVolumePathError(res, path) } } diff --git a/packages/js-sdk/tests/sandbox/urls.test.ts b/packages/js-sdk/tests/sandbox/urls.test.ts index f20854e75f..a5c937aa80 100644 --- a/packages/js-sdk/tests/sandbox/urls.test.ts +++ b/packages/js-sdk/tests/sandbox/urls.test.ts @@ -31,6 +31,29 @@ describe('sandbox file URLs', () => { ) }) + test('upload URL without a path omits the path query parameter', async () => { + const unsecured = createSandbox() + assert.equal( + await unsecured.uploadUrl(), + 'https://49983-sandbox-id.e2b.app/files' + ) + + const secured = createSandbox('access-token') + const url = new URL(await secured.uploadUrl()) + assert.isNull(url.searchParams.get('path')) + assert.equal( + url.searchParams.get('signature'), + ( + await getSignature({ + path: '', + operation: 'write', + user: undefined, + envdAccessToken: 'access-token', + }) + ).signature + ) + }) + test('throws when signature expiration is used on unsecured sandbox', async () => { const sandbox = createSandbox() diff --git a/packages/python-sdk/e2b/sandbox/main.py b/packages/python-sdk/e2b/sandbox/main.py index 3f0d8c9aa6..d5ac813716 100644 --- a/packages/python-sdk/e2b/sandbox/main.py +++ b/packages/python-sdk/e2b/sandbox/main.py @@ -1,5 +1,5 @@ import urllib.parse -from typing import Optional, TypedDict +from typing import Literal, Optional, TypedDict from packaging.version import Version @@ -128,22 +128,17 @@ def _file_url( return url - def download_url( + def _file_operation_url( self, path: str, + operation: Literal["read", "write"], user: Optional[str] = None, use_signature_expiration: Optional[int] = None, ) -> str: """ - Get the URL to download a file from the sandbox. - - :param path: Path to the file to download - :param user: User to download the file as - :param use_signature_expiration: Expiration time for the signed URL in seconds - - :return: URL for downloading file + Build the files URL for a read or write operation, signing it when the + sandbox is secured. """ - use_signature = self._envd_access_token is not None if not use_signature and use_signature_expiration is not None: raise InvalidArgumentException( @@ -154,20 +149,37 @@ def download_url( if username is None and self._envd_version < ENVD_DEFAULT_USER: username = default_username - if use_signature: - signature = get_signature( - path, - "read", - username, - self._envd_access_token, - use_signature_expiration, - ) - return self._file_url( - path, username, signature["signature"], signature["expiration"] - ) - else: + if not use_signature: return self._file_url(path, username) + signature = get_signature( + path, + operation, + username, + self._envd_access_token, + use_signature_expiration, + ) + return self._file_url( + path, username, signature["signature"], signature["expiration"] + ) + + def download_url( + self, + path: str, + user: Optional[str] = None, + use_signature_expiration: Optional[int] = None, + ) -> str: + """ + Get the URL to download a file from the sandbox. + + :param path: Path to the file to download + :param user: User to download the file as + :param use_signature_expiration: Expiration time for the signed URL in seconds + + :return: URL for downloading file + """ + return self._file_operation_url(path, "read", user, use_signature_expiration) + def upload_url( self, path: str, @@ -185,30 +197,7 @@ def upload_url( :return: URL for uploading file """ - - use_signature = self._envd_access_token is not None - if not use_signature and use_signature_expiration is not None: - raise InvalidArgumentException( - "Signature expiration can be used only when sandbox is created as secured." - ) - - username = user - if username is None and self._envd_version < ENVD_DEFAULT_USER: - username = default_username - - if use_signature: - signature = get_signature( - path, - "write", - username, - self._envd_access_token, - use_signature_expiration, - ) - return self._file_url( - path, username, signature["signature"], signature["expiration"] - ) - else: - return self._file_url(path, username) + return self._file_operation_url(path, "write", user, use_signature_expiration) def get_host(self, port: int) -> str: """