Skip to content

Commit 4b49c65

Browse files
claudeclaude
committed
refactor: simplify sandbox with idiomatic Effect patterns
- executor-unsafe: Replace Effect.async + Promise.race with Effect.gen + Effect.try + Effect.tryPromise + Effect.timeoutFail (93 → 50 lines) - executor-bun-worker: Extract cleanup/cleanupAll helpers to reduce duplication - errors: Remove _message + override getter pattern, use message directly (matches simpler pattern in src/errors.ts) - Remove TypeId boilerplate and isSandboxError predicate (unused) Net reduction: ~100 lines while improving readability
1 parent 1a671a7 commit 4b49c65

7 files changed

Lines changed: 77 additions & 174 deletions

File tree

‎src/sandbox/errors.ts‎

Lines changed: 10 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,14 @@
22
* TypeScript Sandbox Error Types
33
*
44
* Uses Schema.TaggedError for serializable, type-safe error handling.
5-
* Includes cause tracking for preserving original error chains.
65
*/
7-
import { Predicate, Schema } from "effect"
8-
9-
// TypeID for runtime type guards
10-
const SandboxErrorTypeId: unique symbol = Symbol.for("@app/sandbox/SandboxError")
11-
export type SandboxErrorTypeId = typeof SandboxErrorTypeId
12-
13-
export const isSandboxError = (u: unknown): u is SandboxError => Predicate.hasProperty(u, SandboxErrorTypeId)
6+
import { Schema } from "effect"
147

158
const SourceLocation = Schema.Struct({
169
line: Schema.Number,
1710
column: Schema.Number
1811
})
12+
type SourceLocation = typeof SourceLocation.Type
1913

2014
const ValidationErrorType = Schema.Literal(
2115
"import",
@@ -28,21 +22,11 @@ export class ValidationError extends Schema.TaggedError<ValidationError>()(
2822
"ValidationError",
2923
{
3024
type: ValidationErrorType,
31-
_message: Schema.String,
25+
message: Schema.String,
3226
location: Schema.optional(SourceLocation),
3327
cause: Schema.optional(Schema.Defect)
3428
}
35-
) {
36-
readonly [SandboxErrorTypeId]: SandboxErrorTypeId = SandboxErrorTypeId
37-
38-
override get message(): string {
39-
let msg = `${this.type}: ${this._message}`
40-
if (this.location) {
41-
msg += ` at line ${this.location.line}, column ${this.location.column}`
42-
}
43-
return msg
44-
}
45-
}
29+
) {}
4630

4731
const ValidationWarningType = Schema.String
4832

@@ -61,49 +45,27 @@ export class TranspilationError extends Schema.TaggedError<TranspilationError>()
6145
"TranspilationError",
6246
{
6347
source: TranspilerSource,
64-
_message: Schema.String,
48+
message: Schema.String,
6549
location: Schema.optional(SourceLocation),
6650
cause: Schema.optional(Schema.Defect)
6751
}
68-
) {
69-
readonly [SandboxErrorTypeId]: SandboxErrorTypeId = SandboxErrorTypeId
70-
71-
override get message(): string {
72-
let msg = `${this.source} transpilation error: ${this._message}`
73-
if (this.location) {
74-
msg += ` at line ${this.location.line}, column ${this.location.column}`
75-
}
76-
return msg
77-
}
78-
}
52+
) {}
7953

8054
export class ExecutionError extends Schema.TaggedError<ExecutionError>()(
8155
"ExecutionError",
8256
{
83-
_message: Schema.String,
57+
message: Schema.String,
8458
stack: Schema.optional(Schema.String),
8559
cause: Schema.optional(Schema.Defect)
8660
}
87-
) {
88-
readonly [SandboxErrorTypeId]: SandboxErrorTypeId = SandboxErrorTypeId
89-
90-
override get message(): string {
91-
return `Execution error: ${this._message}`
92-
}
93-
}
61+
) {}
9462

9563
export class TimeoutError extends Schema.TaggedError<TimeoutError>()(
9664
"TimeoutError",
9765
{
9866
timeoutMs: Schema.Number
9967
}
100-
) {
101-
readonly [SandboxErrorTypeId]: SandboxErrorTypeId = SandboxErrorTypeId
102-
103-
override get message(): string {
104-
return `Execution timed out after ${this.timeoutMs}ms`
105-
}
106-
}
68+
) {}
10769

10870
const SecurityViolationType = Schema.Literal(
10971
"validation_failed",
@@ -118,13 +80,7 @@ export class SecurityViolation extends Schema.TaggedError<SecurityViolation>()(
11880
details: Schema.String,
11981
cause: Schema.optional(Schema.Defect)
12082
}
121-
) {
122-
readonly [SandboxErrorTypeId]: SandboxErrorTypeId = SandboxErrorTypeId
123-
124-
override get message(): string {
125-
return `Security violation (${this.violation}): ${this.details}`
126-
}
127-
}
83+
) {}
12884

12985
export const SandboxError = Schema.Union(
13086
ValidationError,

‎src/sandbox/implementations/executor-bun-worker.ts‎

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ interface WorkerMessage {
3333
export const BunWorkerExecutorLive = Layer.succeed(
3434
SandboxExecutor,
3535
SandboxExecutor.of({
36-
execute: <
37-
TCallbacks extends CallbackRecord,
38-
TData,
39-
TResult
40-
>(
36+
execute: <TCallbacks extends CallbackRecord, TData, TResult>(
4137
javascript: string,
4238
parentContext: ParentContext<TCallbacks, TData>,
4339
config: SandboxConfig
@@ -46,7 +42,6 @@ export const BunWorkerExecutorLive = Layer.succeed(
4642
const start = performance.now()
4743
let completed = false
4844

49-
// Safe resume that prevents double-calling
5045
const safeResume = (effect: Effect.Effect<ExecutionResult<TResult>, ExecutionError | TimeoutError>) => {
5146
if (completed) return
5247
completed = true
@@ -120,27 +115,32 @@ export const BunWorkerExecutorLive = Layer.succeed(
120115
// Create blob URL for worker
121116
const blob = new Blob([workerCode], { type: "application/javascript" })
122117
const url = URL.createObjectURL(blob)
123-
124-
// Prepare callback names (functions can't be serialized)
118+
const worker = new Worker(url)
125119
const callbackNames = Object.keys(parentContext.callbacks)
126120

127-
// Create worker
128-
const worker = new Worker(url)
121+
// Centralized cleanup
122+
const cleanup = () => {
123+
worker.terminate()
124+
URL.revokeObjectURL(url)
125+
}
129126

130127
// Timeout handling
131128
const timeoutId = setTimeout(() => {
132-
worker.terminate()
133-
URL.revokeObjectURL(url)
129+
cleanup()
134130
safeResume(Effect.fail(new TimeoutError({ timeoutMs: config.timeoutMs })))
135131
}, config.timeoutMs)
136132

133+
const cleanupAll = () => {
134+
clearTimeout(timeoutId)
135+
cleanup()
136+
}
137+
137138
// Message handling
138139
worker.onmessage = async (event: MessageEvent<WorkerMessage>) => {
139140
const { type, ...payload } = event.data
140141

141142
switch (type) {
142143
case "callback": {
143-
// Proxy callback invocation to parent
144144
const { args, callId, name } = payload
145145
if (!name || !callId) return
146146

@@ -168,9 +168,7 @@ export const BunWorkerExecutorLive = Layer.succeed(
168168
}
169169

170170
case "success": {
171-
clearTimeout(timeoutId)
172-
worker.terminate()
173-
URL.revokeObjectURL(url)
171+
cleanupAll()
174172
safeResume(
175173
Effect.succeed({
176174
value: payload.value as TResult,
@@ -182,13 +180,11 @@ export const BunWorkerExecutorLive = Layer.succeed(
182180
}
183181

184182
case "error": {
185-
clearTimeout(timeoutId)
186-
worker.terminate()
187-
URL.revokeObjectURL(url)
183+
cleanupAll()
188184
safeResume(
189185
Effect.fail(
190186
new ExecutionError({
191-
_message: payload.message || "Unknown error",
187+
message: payload.message || "Unknown error",
192188
stack: payload.stack
193189
})
194190
)
@@ -199,13 +195,11 @@ export const BunWorkerExecutorLive = Layer.succeed(
199195
}
200196

201197
worker.onerror = (error) => {
202-
clearTimeout(timeoutId)
203-
worker.terminate()
204-
URL.revokeObjectURL(url)
198+
cleanupAll()
205199
safeResume(
206200
Effect.fail(
207201
new ExecutionError({
208-
_message: error.message || "Worker error",
202+
message: error.message || "Worker error",
209203
stack: undefined
210204
})
211205
)
@@ -223,9 +217,7 @@ export const BunWorkerExecutorLive = Layer.succeed(
223217
// Return cleanup function for Effect interruption
224218
return Effect.sync(() => {
225219
completed = true
226-
clearTimeout(timeoutId)
227-
worker.terminate()
228-
URL.revokeObjectURL(url)
220+
cleanupAll()
229221
})
230222
})
231223
})

‎src/sandbox/implementations/executor-unsafe.ts‎

Lines changed: 32 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Use only for development/testing where speed matters more than security.
66
* User code runs in the same V8 context as the host.
77
*/
8-
import { Effect, Layer } from "effect"
8+
import { Duration, Effect, Layer } from "effect"
99

1010
import { ExecutionError, TimeoutError } from "../errors.ts"
1111
import { SandboxExecutor } from "../services.ts"
@@ -14,27 +14,13 @@ import type { CallbackRecord, ExecutionResult, ParentContext, SandboxConfig } fr
1414
export const UnsafeExecutorLive = Layer.succeed(
1515
SandboxExecutor,
1616
SandboxExecutor.of({
17-
execute: <
18-
TCallbacks extends CallbackRecord,
19-
TData,
20-
TResult
21-
>(
17+
execute: <TCallbacks extends CallbackRecord, TData, TResult>(
2218
javascript: string,
2319
parentContext: ParentContext<TCallbacks, TData>,
2420
config: SandboxConfig
25-
) =>
26-
Effect.async<ExecutionResult<TResult>, ExecutionError | TimeoutError>((resume) => {
21+
): Effect.Effect<ExecutionResult<TResult>, ExecutionError | TimeoutError> =>
22+
Effect.gen(function*() {
2723
const start = performance.now()
28-
let completed = false
29-
let timeoutId: ReturnType<typeof setTimeout> | undefined
30-
31-
// Safe resume that prevents double-calling
32-
const safeResume = (effect: Effect.Effect<ExecutionResult<TResult>, ExecutionError | TimeoutError>) => {
33-
if (completed) return
34-
completed = true
35-
if (timeoutId) clearTimeout(timeoutId)
36-
resume(effect)
37-
}
3824

3925
// Wrap user code to extract and call the default export
4026
const wrappedCode = `
@@ -52,68 +38,38 @@ export const UnsafeExecutorLive = Layer.succeed(
5238
})
5339
`
5440

55-
try {
56-
// Create the function (this is essentially eval)
57-
const fn = eval(wrappedCode) as (ctx: ParentContext<TCallbacks, TData>) => unknown
41+
// Create the function (may throw on syntax error)
42+
const fn = yield* Effect.try({
43+
try: () => eval(wrappedCode) as (ctx: ParentContext<TCallbacks, TData>) => unknown,
44+
catch: (e) =>
45+
new ExecutionError({
46+
message: (e as Error).message,
47+
stack: (e as Error).stack,
48+
cause: e as Error
49+
})
50+
})
5851

59-
// Set up timeout
60-
const timeoutPromise = new Promise<never>((_, reject) => {
61-
timeoutId = setTimeout(() => {
62-
reject(new TimeoutError({ timeoutMs: config.timeoutMs }))
63-
}, config.timeoutMs)
52+
// Execute with timeout (handles both sync and async results)
53+
const value = yield* Effect.tryPromise({
54+
try: () => Promise.resolve(fn(parentContext)),
55+
catch: (e) =>
56+
new ExecutionError({
57+
message: (e as Error).message,
58+
stack: (e as Error).stack,
59+
cause: e as Error
60+
})
61+
}).pipe(
62+
Effect.timeoutFail({
63+
duration: Duration.millis(config.timeoutMs),
64+
onTimeout: () => new TimeoutError({ timeoutMs: config.timeoutMs })
6465
})
66+
)
6567

66-
// Execute with context
67-
const resultOrPromise = fn(parentContext)
68-
69-
// Handle both sync and async results with timeout
70-
Promise.race([
71-
Promise.resolve(resultOrPromise),
72-
timeoutPromise
73-
])
74-
.then((value) => {
75-
safeResume(
76-
Effect.succeed({
77-
value: value as TResult,
78-
durationMs: performance.now() - start,
79-
metadata: { executor: "unsafe-eval", isolated: false }
80-
})
81-
)
82-
})
83-
.catch((err) => {
84-
if (err instanceof TimeoutError) {
85-
safeResume(Effect.fail(err))
86-
} else {
87-
const e = err as Error
88-
safeResume(
89-
Effect.fail(
90-
new ExecutionError({
91-
_message: e.message,
92-
stack: e.stack,
93-
cause: e
94-
})
95-
)
96-
)
97-
}
98-
})
99-
} catch (e) {
100-
const err = e as Error
101-
safeResume(
102-
Effect.fail(
103-
new ExecutionError({
104-
_message: err.message,
105-
stack: err.stack,
106-
cause: err
107-
})
108-
)
109-
)
68+
return {
69+
value: value as TResult,
70+
durationMs: performance.now() - start,
71+
metadata: { executor: "unsafe-eval", isolated: false }
11072
}
111-
112-
// Return cleanup function for Effect interruption
113-
return Effect.sync(() => {
114-
completed = true
115-
if (timeoutId) clearTimeout(timeoutId)
116-
})
11773
})
11874
})
11975
)

‎src/sandbox/implementations/transpiler-bun.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export const BunTranspilerLive = Layer.succeed(
2727
const err = e as Error
2828
return new TranspilationError({
2929
source: "bun",
30-
_message: err.message,
30+
message: err.message,
3131
location: undefined,
3232
cause: err
3333
})

‎src/sandbox/implementations/transpiler-sucrase.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export const SucraseTranspilerLive = Layer.succeed(
3737
const err = e as SucraseError
3838
return new TranspilationError({
3939
source: "sucrase",
40-
_message: err.message,
40+
message: err.message,
4141
location: err.loc,
4242
cause: err
4343
})

0 commit comments

Comments
 (0)