Skip to content

Commit

Permalink
fix: improve DX upon wrong HTTP request body
Browse files Browse the repository at this point in the history
  • Loading branch information
brillout committed Feb 3, 2022
1 parent 3341474 commit f8751f0
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 34 deletions.
2 changes: 1 addition & 1 deletion telefunc/client/utils/hasProp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ function hasProp<ObjectType, PropName extends PropertyKey>(obj: ObjectType, prop
if( !propExists ){
return false
}
const propValue = (obj as Record<any,unknown>)[prop]
if( type === 'unknown' ) {
return true
}
const propValue = (obj as Record<any,unknown>)[prop]
if( type === 'array') {
return Array.isArray(propValue)
}
Expand Down
16 changes: 2 additions & 14 deletions telefunc/node/server/runTelefunc/assertHttpRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,6 @@ function assertHttpRequest(httpRequest: unknown, numberOfArguments: number) {
assertUsage(hasProp(httpRequest, 'url', 'string'), '`telefunc({ url })`: argument `url` should be a string.')
assertUsage(hasProp(httpRequest, 'method'), '`telefunc({ method })`: argument `method` is missing.')
assertUsage(hasProp(httpRequest, 'method', 'string'), '`telefunc({ method })`: argument `method` should be a string.')
assertUsage('body' in httpRequest, '`telefunc({ body })`: argument `body` is missing.')
const { body } = httpRequest
assertUsage(
body !== undefined && body !== null,
'`telefunc({ body })`: argument `body` should be a string or an object but `body === ' +
body +
'`. Note that with some server frameworks, such as Express.js, you need to use a server middleware that parses the body.',
)
assertUsage(
typeof body === 'string' || isObject(body),
"`telefunc({ body })`: argument `body` should be a string or an object but `typeof body === '" + typeof body + "'`",
)
checkType<HttpRequest['body']>(body)
checkType<Omit<HttpRequest, 'body'>>(httpRequest)
assertUsage(hasProp(httpRequest, 'body'), '`telefunc({ body })`: argument `body` is missing.')
checkType<HttpRequest>(httpRequest)
}
79 changes: 64 additions & 15 deletions telefunc/node/server/runTelefunc/parseHttpRequest.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
export { parseHttpRequest }

import { parse } from '@brillout/json-s/parse'
import { assertUsage, hasProp } from '../../utils'
import { assertUsage, hasProp, getPluginError } from '../../utils'
import { getTelefunctionKey } from './getTelefunctionKey'

function parseHttpRequest(runContext: { httpRequest: { body: string | object }; isProduction: boolean }):
function parseHttpRequest(runContext: { httpRequest: { body: unknown }; isProduction: boolean; telefuncUrl: string }):
| {
telefunctionFilePath: string
telefunctionExportName: string
Expand All @@ -14,31 +14,56 @@ function parseHttpRequest(runContext: { httpRequest: { body: string | object };
}
| { isMalformed: true } {
const { body } = runContext.httpRequest
const bodyString = typeof body === 'string' ? body : JSON.stringify(body)
if (typeof body !== 'string') {
if (!runContext.isProduction) {
// In production `body` can be any value really.
// Therefore we `assertBody(body)` only development.
assertBody(body, runContext)
}
return { isMalformed: true }
}
const bodyString: string = body

const devErrMsgPrefix =
'Malformed request in development. This is unexpected since, in development, all requests are expected to originate from the Telefunc Client. If this error is happening in production, then this means that you forgot to set the environment variable `NODE_ENV="production"` or `telefunc({ isProduction: true })`.'

let bodyParsed: unknown
try {
bodyParsed = parse(bodyString)
} catch (err_) {}
} catch (err: unknown) {
if (!runContext.isProduction) {
console.error(
getPluginError(
[
devErrMsgPrefix,
`Following body string could not be parsed: \`${bodyString}\`.`,
!hasProp(err, 'message') ? null : 'Parse error: ' + err.message,
]
.filter(Boolean)
.join(' '),
),
)
}
return { isMalformed: true }
}

if (
!hasProp(bodyParsed, 'file', 'string') ||
!hasProp(bodyParsed, 'name', 'string') ||
!hasProp(bodyParsed, 'args', 'array')
) {
if (runContext.isProduction) {
// In production, a third party can make a malformed request.
return { isMalformed: true }
} else {
// If in development, then something is wrong
assertUsage(
false,
'`telefunc({ body })`: argument `body` should be the body of the HTTP request. This is not the case; make sure you are properly retrieving the HTTP request body and pass it to `telefunc({ body })`. ' +
'(Parsed `body`: `' +
JSON.stringify(bodyParsed) +
'`.)',
if (!runContext.isProduction) {
console.error(
getPluginError(
[
devErrMsgPrefix,
'The `body` argument passed to `telefunc({ body })` is not valid',
`(\`body === '${bodyString}'\`).`,
].join(' '),
),
)
}
return { isMalformed: true }
}

const telefunctionFilePath = bodyParsed.file
Expand All @@ -54,3 +79,27 @@ function parseHttpRequest(runContext: { httpRequest: { body: string | object };
isMalformed: false,
}
}

function assertBody(body: unknown, runContext: { telefuncUrl: string }) {
const errorNote = [
`Make sure that \`body\` is the HTTP body of the request HTTP POST \`Content-Type: text/plain\` \`${runContext.telefuncUrl}\`.`,
'Note that with some server frameworks, such as Express.js, you need to use a server middleware to process the HTTP body of `Content-Type: text/plain` requests.',
].join(' ')
assertUsage(
body !== undefined && body !== null,
['`telefunc({ body })`: argument `body` should be a string but', `\`body === ${body}\`.`, errorNote].join(' '),
)
assertUsage(
typeof body === 'string',
[
'`telefunc({ body })`: argument `body` should be a string but',
`\`typeof body === '${typeof body}'\`.`,
errorNote,
].join(' '),
)
assertUsage(
// Express.js sets `req.body === '{}'` upon wrong Content-Type
body !== '{}',
["`telefunc({ body })`: argument `body` is an empty JSON object (`body === '{}'`).", errorNote].join(' '),
)
}
8 changes: 5 additions & 3 deletions telefunc/node/server/telefunc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export { telefunc }
import { assertHttpRequest } from './runTelefunc/assertHttpRequest'
import { runTelefunc } from './runTelefunc'
import { HttpRequest } from './types'
import { assertUsage, getUrlPathname, objectAssign } from '../utils'
import { assert, assertUsage, getUrlPathname, objectAssign } from '../utils'
import { telefuncConfig } from './telefuncConfig'

/**
Expand All @@ -13,7 +13,7 @@ import { telefuncConfig } from './telefuncConfig'
* @param httpRequest.body HTTP request body
* @returns HTTP response
*/
function telefunc(httpRequest: HttpRequest) {
async function telefunc(httpRequest: HttpRequest) {
assertHttpRequest(httpRequest, arguments.length)

const runContext = {}
Expand All @@ -22,7 +22,9 @@ function telefunc(httpRequest: HttpRequest) {

assertUrl(runContext)

return runTelefunc(runContext)
const httpResponse = await runTelefunc(runContext)
assert(httpResponse)
return httpResponse
}

function assertUrl(runContext: { httpRequest: { url: string }; telefuncUrl: string }) {
Expand Down
2 changes: 1 addition & 1 deletion telefunc/node/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type Telefunction = (...args: unknown[]) => Promise<unknown>
type HttpRequest = {
url: string
method: string
body: string | object
body: unknown
}

type FileExports = Record<string, unknown>
Expand Down

0 comments on commit f8751f0

Please sign in to comment.