Skip to content

Commit

Permalink
Terminal: show last error description on timeout (#1810)
Browse files Browse the repository at this point in the history
* [prefactor] until condition returns {ok[, object]}

* [prefactor] until condition returns optional reason

* terminal: show last error description on timeout
  • Loading branch information
petersutter authored Apr 18, 2024
1 parent 5cece4e commit 69c3963
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 15 deletions.
4 changes: 3 additions & 1 deletion backend/lib/services/projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ exports.create = async function ({ user, body }) {
if (type === 'DELETE') {
throw new InternalServerError('Project resource has been deleted')
}
return _.get(project, 'status.phase') === 'Ready'
return {
ok: _.get(project, 'status.phase') === 'Ready'
}
}
const timeout = exports.projectInitializationTimeout
// must be the dashboardClient because rbac rolebinding does not exist yet
Expand Down
12 changes: 11 additions & 1 deletion backend/lib/services/terminals/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,17 @@ async function readTerminalUntilReady ({ user, namespace, name }) {
}
const podName = _.get(terminal, 'status.podName')
const attachServiceAccountName = _.get(terminal, 'status.attachServiceAccountName')
return podName && attachServiceAccountName

const isReady = podName && attachServiceAccountName
if (!isReady) {
const lastErrorDescription = _.get(terminal, 'status.lastError.description')
return {
ok: false,
reason: lastErrorDescription
}
}

return { ok: true }
}
const asyncIterable = await client['dashboard.gardener.cloud'].terminals.watch(namespace, name)
return asyncIterable.until(isTerminalReady, { timeout: 60 * 1000 })
Expand Down
6 changes: 1 addition & 5 deletions frontend/src/components/GTerminal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ import GDisconnected from '@/components/icons/GDisconnected.vue'
import GSplitVertically from '@/components/icons/GSplitVertically.vue'
import GSplitHorizontally from '@/components/icons/GSplitHorizontally.vue'

import { isGatewayTimeout } from '@/utils/error'
import {
targetText,
transformHtml,
Expand Down Expand Up @@ -683,10 +682,7 @@ export default {
try {
await terminalSession.open()
} catch (err) {
let message = get(err, 'response.data.message', err.message)
if (isGatewayTimeout(err)) {
message = 'Opening the terminal session timed out'
}
const message = get(err, 'response.data.message', err.message)
this.showErrorSnackbarBottom(message)
terminalSession.setDisconnectedState()
}
Expand Down
10 changes: 8 additions & 2 deletions packages/kube-client/__tests__/http-client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ describe('kube-client', () => {
expect(mockClient.stream).toBeCalledTimes(1)
expect(mockClient.stream.mock.calls[0]).toEqual([url, { method }])
expect(typeof response.until).toBe('function')
const value = await response.until(event => [event.object > 3, event.object])
const value = await response.until(event => ({
ok: event.object > 3,
object: event.object
}))
expect(value).toBe(4)
})

Expand All @@ -85,7 +88,10 @@ describe('kube-client', () => {
expect(mockClient.stream).toBeCalledTimes(1)
expect(mockClient.stream.mock.calls[0]).toEqual([url, { method }])
expect(typeof response.until).toBe('function')
await expect(response.until(event => event.object > 9, { timeout: 10 })).rejects.toThrow(GatewayTimeout)
await expect(response.until(
event => ({
ok: event.object > 9
}), { timeout: 10 })).rejects.toThrow(GatewayTimeout)
expect(destroySpy).toBeCalled()
expect(destroySpy.mock.calls[0]).toHaveLength(1)
expect(destroySpy.mock.calls[0][0].message).toMatch(/"dummies" .* 10 ms/)
Expand Down
21 changes: 15 additions & 6 deletions packages/kube-client/lib/HttpClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,33 @@ class HttpClient {

static extendResponse (response) {
const { names: { plural } = {} } = this
const createTimeoutError = timeout => {
const createTimeoutError = (timeout, reason) => {
const forResource = plural ? ` for "${plural}"` : ''
return createError(504, `The condition${forResource} was not met within ${timeout} ms`)
let message = `The condition${forResource} was not met within ${timeout} ms`
if (reason) {
message += `: ${reason}`
}
return createError(504, message)
}

response.until = async (condition, { timeout = 60000 } = {}) => {
let timeoutId
let timeoutReason
if (timeout > 0 && timeout < Infinity) {
timeoutId = setTimeout(() => response.destroy(createTimeoutError(timeout)), timeout)
timeoutId = setTimeout(() => response.destroy(createTimeoutError(timeout, timeoutReason)), timeout)
}
try {
for await (const event of response) {
let ok = condition(event)
let object
[ok, object] = Array.isArray(ok) ? ok : [ok, event.object]
const {
ok,
reason,
object = event.object
} = condition(event)
if (ok) {
return object
}

timeoutReason = reason
}
// If the response stream ends even though the condition has not yet been met,
// also in this case a timeout error is thrown.
Expand Down

0 comments on commit 69c3963

Please sign in to comment.