Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test new error handling indicating when the sandbox is not found #540

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
37855b9
add host test for sandbox not found error
0div Jan 16, 2025
91770ae
skip to not block wokflow
0div Jan 16, 2025
55ecbaf
Update packages/js-sdk/tests/sandbox/host.test.ts
0div Jan 16, 2025
c00f73f
skip ifdebug
0div Jan 16, 2025
c96e4de
merge upstream
0div Jan 16, 2025
ad3d8fb
skip ifdebug
0div Jan 16, 2025
a9c283e
edits based on feedback
0div Jan 17, 2025
399f929
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Jan 18, 2025
7cc525e
Merge branch 'main' of https://github.com/e2b-dev/E2B into return-err…
0div Jan 18, 2025
06ef257
address PR comments
0div Jan 18, 2025
cb0a3a0
Merge branch 'main' of https://github.com/e2b-dev/E2B into return-err…
0div Jan 20, 2025
a7606f5
Apply suggestions from code review
jakubno Jan 20, 2025
3ecbc9f
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Jan 21, 2025
21858ad
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Jan 25, 2025
febf55a
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Jan 27, 2025
f5cde50
fix type in host test
0div Jan 27, 2025
4540c86
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Jan 28, 2025
1823bd7
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Jan 29, 2025
c16bb4d
fixe merge conflict with main
0div Mar 18, 2025
7923032
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Mar 20, 2025
62b4a47
Merge branch 'main' into return-error-indicating-that-the-sandbox-is-…
0div Mar 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/js-sdk/tests/sandbox/commands/sendStdin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ sandboxTest('send stdin to process', async ({ sandbox }) => {

await sandbox.commands.sendStdin(cmd.pid, text)


for (let i = 0; i < 5; i++) {
if (cmd.stdout === text) {
break
Expand Down Expand Up @@ -64,6 +63,5 @@ sandboxTest('send multiline string to stdin', async ({ sandbox }) => {

await cmd.kill()


assert.equal(cmd.stdout, text)
})
24 changes: 23 additions & 1 deletion packages/js-sdk/tests/sandbox/connect.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, assert } from 'vitest'
import { assert, test } from 'vitest'

import { Sandbox } from '../../src'
import { isDebug, template } from '../setup.js'
Expand All @@ -19,3 +19,25 @@ test('connect', async () => {
}
}
})

test('connect to non-running sandbox', async () => {
const sbx = await Sandbox.create(template, { timeoutMs: 10_000 })
let isKilled = false

try {
const isRunning = await sbx.isRunning()
assert.isTrue(isRunning)
await sbx.kill()
isKilled = true

const sbxConnection = await Sandbox.connect(sbx.sandboxId)
const isRunning2 = await sbxConnection.isRunning()
assert.isFalse(isRunning2)

await sbxConnection.commands.run('echo "hello"')
} finally {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this run command always fail? It looks to me like there is a missing assert on the behavior what is expected here

if (!isKilled) {
await sbx.kill()
}
}
})
66 changes: 46 additions & 20 deletions packages/js-sdk/tests/sandbox/host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,56 @@ import { assert } from 'vitest'

import { isDebug, sandboxTest, wait } from '../setup.js'

sandboxTest('ping server in sandbox', async ({ sandbox }) => {
const cmd = await sandbox.commands.run('python -m http.server 8000', { background: true })
sandboxTest.skipIf(isDebug)(
'ping server in running sandbox',
async ({ sandbox }) => {
const cmd = await sandbox.commands.run('python -m http.server 8000', {
background: true,
})

try {
await wait(1000)
try {
await wait(1000)

const host = sandbox.getHost(8000)
const host = sandbox.getHost(8000)

let res = await fetch(`${isDebug ? 'http' : 'https'}://${host}`)
let res = await fetch(`${isDebug ? 'http' : 'https'}://${host}`)

for (let i = 0; i < 20; i++) {
if (res.status === 200) {
break
}
for (let i = 0; i < 20; i++) {
if (res.status === 200) {
break
}

res = await fetch(`${isDebug ? 'http' : 'https'}://${host}`)
await wait(500)
}
assert.equal(res.status, 200)
} finally {
try {
await cmd.kill()
} catch (e) {
console.error(e)
res = await fetch(`${isDebug ? 'http' : 'https'}://${host}`)
await wait(500)
}
assert.equal(res.status, 200)
} finally {
try {
await cmd.kill()
} catch (e) {
console.error(e)
}
}
},
60_000
)

sandboxTest.skipIf(isDebug)(
'ping server in non-running sandbox',
async ({ sandbox }) => {
const host = sandbox.getHost(49983)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the port would be better in a constant

const url = `${isDebug ? 'http' : 'https'}://${host}/health`

const res = await fetch(url)

assert.equal(res.status, 204)

await sandbox.kill()

const res2 = await fetch(url)
assert.equal(res2.status, 502)

const text = await res2.text()
assert.equal(text, 'Sandbox not found')
}
}, 60_000 )
)
Loading