Skip to content

Commit

Permalink
fix: improve DX upon server errors
Browse files Browse the repository at this point in the history
  • Loading branch information
brillout committed Oct 24, 2022
1 parent dcf1423 commit 4321598
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 37 deletions.
10 changes: 2 additions & 8 deletions telefunc/client/remoteTelefunctionCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export { remoteTelefunctionCall }
import { makeHttpRequest } from './remoteTelefunctionCall/makeHttpRequest'
import { serializeTelefunctionArguments } from './remoteTelefunctionCall/serializeTelefunctionArguments'
import { telefuncConfig } from './telefuncConfig'
import { objectAssign, assertUsage, isBrowser, assert } from './utils'
import { objectAssign, assertUsage, isBrowser } from './utils'

async function remoteTelefunctionCall(
telefuncFilePath: string,
Expand All @@ -28,12 +28,6 @@ async function remoteTelefunctionCall(
objectAssign(callContext, { httpRequestBody })
}

const resp = await makeHttpRequest(callContext)
if ('telefunctionCallError' in resp) {
const { telefunctionCallError } = resp
assert(telefunctionCallError)
throw telefunctionCallError
}
const { telefunctionReturn } = resp
const { telefunctionReturn } = await makeHttpRequest(callContext)
return telefunctionReturn
}
20 changes: 9 additions & 11 deletions telefunc/client/remoteTelefunctionCall/makeHttpRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ export { makeHttpRequest }
import { parse } from '@brillout/json-s/parse'
import { assert, assertUsage, isObject, objectAssign } from '../utils'
import { executeCallErrorListeners } from './onTelefunctionRemoteCallError'
import type { TelefunctionError } from '../TelefunctionError'

const method = 'POST'
const STATUS_CODE_SUCCESS = 200
Expand All @@ -17,7 +16,7 @@ async function makeHttpRequest(callContext: {
telefunctionName: string
telefuncFilePath: string
httpHeaders: Record<string, string>
}): Promise<{ telefunctionReturn: unknown } | { telefunctionCallError: TelefunctionError }> {
}): Promise<{ telefunctionReturn: unknown }> {
let response: Response
try {
response = await fetch(callContext.telefuncUrl, {
Expand All @@ -33,7 +32,7 @@ async function makeHttpRequest(callContext: {
const telefunctionCallError = new Error('No Server Connection')
objectAssign(telefunctionCallError, { isConnectionError: true as const })
executeCallErrorListeners(telefunctionCallError)
return { telefunctionCallError }
throw telefunctionCallError
}

const statusCode = response.status
Expand All @@ -50,32 +49,31 @@ async function makeHttpRequest(callContext: {
)
objectAssign(telefunctionCallError, { isAbort: true as const, abortValue })
executeCallErrorListeners(telefunctionCallError)
return { telefunctionCallError }
throw telefunctionCallError
} else if (statusCode === STATUS_CODE_BUG) {
const responseBody = await response.text()
const errMsg = 'Internal Server Error'
assertUsage(
responseBody === 'Internal Server Error (Telefunc)',
responseBody === errMsg,
installErr({
reason: 'an HTTP response body that Telefunc never generates',
method,
callContext
})
)
const telefunctionCallError = new Error('Server Error')
return { telefunctionCallError }
throw new Error(errMsg)
} else if (statusCode === STATUS_CODE_INVALID) {
const responseBody = await response.text()
assertUsage(
responseBody === 'Invalid Request (Telefunc)',
responseBody === 'Invalid Telefunc Request',
installErr({
reason: 'an HTTP response body that Telefunc never generates',
method,
callContext
})
)
// In theory this error should never happen.
const telefunctionCallError = new Error('Invalid Telefunc Request')
return { telefunctionCallError }
// This should never happen as the Telefunc Client shouldn't make invalid requests
assert(false)
} else {
assertUsage(
statusCode !== 404,
Expand Down
10 changes: 5 additions & 5 deletions telefunc/node/server/runTelefunc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@ const abortedRequestStatusCode = 403

// HTTP Response for:
// - User's telefunction threw an error (that isn't `Abort()`).
// - Telefunc throw an error (i.e. Telefunc has a bug).
// - The Telefunc code throw an error (i.e. Telefunc has a bug).
// - The telefunction couldn't be found (i.e. Telefunc has a bug or user didn't define any telefunction).
const serverError = {
statusCode: 500 as const,
body: 'Internal Server Error (Telefunc)',
body: 'Internal Server Error',
contentType: 'text/plain' as const,
etag: null
}

// HTTP Response for:
// - The telefunction couldn't be found.
// - Some non-telefunc client makes a malformed HTTP request.
const invalidRequest = {
statusCode: 400 as const,
body: 'Invalid Request (Telefunc)',
body: 'Invalid Telefunc Request',
contentType: 'text/plain' as const,
etag: null
}
Expand Down Expand Up @@ -92,7 +92,7 @@ async function runTelefunc_(httpRequest: { url: string; method: string; body: un
{
const telefunction = findTelefunction(runContext)
if (!telefunction) {
return invalidRequest
return serverError
}
objectAssign(runContext, { telefunction })
}
Expand Down
34 changes: 21 additions & 13 deletions telefunc/node/server/runTelefunc/findTelefunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ function findTelefunction(runContext: {
return telefunction

function getNotFoundErrMsg() {
let errMsg = `Telefunction ${runContext.telefunctionName}() (${runContext.telefuncFilePath}) not found:`
let errMsg = `Telefunction ${runContext.telefunctionName}() (${runContext.telefuncFilePath}) not found: `
if (!telefuncFile) {
errMsg += ` the file \`${runContext.telefuncFilePath}\` doesn't seem to exist. Found \`.telefunc.js\` files:`
errMsg += `the file ${runContext.telefuncFilePath} doesn't exist. Found \`.telefunc.js\` files:`
assert(!runContext.telefuncFilesAll.includes(runContext.telefuncFilePath))
errMsg += [runContext.telefuncFilePath, ...runContext.telefuncFilesAll]
.sort()
Expand All @@ -56,18 +56,26 @@ function findTelefunction(runContext: {
.join('')
} else {
assert(!telefuncFile.fileExports[runContext.telefunctionName])
errMsg += ` the file \`${runContext.telefuncFilePath}\` doesn't seem to have an export a telefunction \`${runContext.telefunctionName}\`. Found telefunctions:`
assert(telefuncFile.filePath === runContext.telefuncFilePath)
errMsg += `the file ${telefuncFile.filePath} doesn't export a telefunction named "${runContext.telefunctionName}". `
const telefuncFileExportNames = Object.keys(telefuncFile.fileExports)
assert(!telefuncFileExportNames.includes(runContext.telefunctionName))
errMsg += [runContext.telefunctionName, ...telefuncFileExportNames]
.sort()
.map(
(exportName) =>
`\n export { ${exportName} } in ${telefuncFile.filePath} ${
telefuncFileExportNames.includes(exportName) ? '[✅ Exists]' : "[❌ Doesn't exist]"
}`
)
.join('')
if (telefuncFileExportNames.length === 0) {
errMsg += `(The file ${telefuncFile.filePath} doesn't export any telefunction.)`
} else {
errMsg += 'Found telefunctions:'
assert(!telefuncFileExportNames.includes(runContext.telefunctionName))
errMsg += [runContext.telefunctionName, ...telefuncFileExportNames]
.sort()
.map(
(exportName) =>
`\n ${telefuncFile.filePath} ${
telefuncFileExportNames.includes(exportName)
? `exports telefunction ${exportName}() ✅`
: `doesn't have an export "${exportName}" ❌`
}`
)
.join('')
}
}
return errMsg
}
Expand Down
1 change: 1 addition & 0 deletions telefunc/node/vite/loadTelefuncFilesWithVite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,6 @@ async function loadGlobFiles(telefuncFilesGlob: GlobFiles, telefuncFilePath: str
.map(async ([filePath, loadModuleExports]) => [filePath, await loadModuleExports()])
)
)
assert(Object.keys(telefuncFilesLoaded).length <= 1)
return { telefuncFilesAll, telefuncFilesLoaded }
}

0 comments on commit 4321598

Please sign in to comment.