Skip to content

Commit

Permalink
Tighten API contracts and unify error handling/reporting so we get mo…
Browse files Browse the repository at this point in the history
…re informative logs
  • Loading branch information
brundonsmith committed Nov 10, 2023
1 parent 0672efb commit 3a9c2be
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 84 deletions.
43 changes: 19 additions & 24 deletions back-end/index.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,48 @@
import { Application, isHttpError, isErrorStatus, STATUS_TEXT } from 'oak'
import { Application, isHttpError, isErrorStatus, STATUS_TEXT, Response } from 'oak'
import { router } from './routes/index.ts'
import { ResponseWithError } from './routes/v1/_common.ts'
import { pad, indent } from './utils/misc.ts'

const app = new Application()

// Overengineering this a little to reduce allocations...
const spacesStrCache = new Map<number, string>()
const spaces = (length: number): string => {
const cached = spacesStrCache.get(length)
// --- Middleware ---

if (cached != null) {
return cached
} else {
const str = new Array(length).fill(' ').join('')
spacesStrCache.set(length, str)
return str
}
}

const pad = (str: string, length: number) => {
const spacesToAdd = Math.max(length - str.length, 0)
return str + spaces(spacesToAdd)
}

// middleware
// log all requests, including errors as applicable
app.use(async (ctx, next) => {
await next()

const baseLog = `[ ${new Date().toISOString()} ${pad(ctx.request.method, 7)} ${pad(ctx.request.url.pathname, 34)} ]: ${ctx.response.status} ${STATUS_TEXT[ctx.response.status]}`
const error = (ctx.response as ResponseWithError).error

if (isErrorStatus(ctx.response.status)) {
console.error(baseLog + '\t' + '<TODO error>')
if (error) {
console.error(baseLog + '\n' + indent(error))
} else {
console.info(baseLog)
}
})


// catch thrown errors and convert them to 500 responses
app.use(async (ctx, next) => {
try {
await next()
} catch (err) {
} catch (err: unknown) {
ctx.response.body = 'null'
ctx.response.type = 'json'

if (isHttpError(err)) {
ctx.response.status = err.status
} else {
ctx.response.status = 500
}

if (err instanceof Error) {
Error.captureStackTrace(err);
(ctx.response as ResponseWithError).error = err.stack
}
}
})

// set CORS headers
app.use(async (ctx, next) => {
ctx.response.headers.set('Access-Control-Allow-Credentials', 'true')
ctx.response.headers.set('Access-Control-Allow-Headers', 'Authorization')
Expand Down
13 changes: 6 additions & 7 deletions back-end/routes/v1/_common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
RouterContext,
RouterMiddleware,
Status,
Response,
} from 'oak'
import { getJwtPayload } from './auth.ts'
import { wait } from '../../utils/misc.ts'
Expand All @@ -25,7 +26,7 @@ export const API_BASE = '/api/v1'
type RouteResponse<TResult> = Promise<[(TResult) | null, Status]>

type UnauthenticatedRouteContext<TEndpoint extends keyof Routes> = {
ctx: AnyRouterContext
ctx: AnyRouterContext & { response: ResponseWithError }
body: Routes[TEndpoint]['body']
}

Expand All @@ -35,6 +36,8 @@ type AuthenticatedRouteContext<TEndpoint extends keyof Routes> = Unauthenticated

type AnyRouteContext = UnauthenticatedRouteContext<keyof Routes> & Partial<AuthenticatedRouteContext<keyof Routes>>

export type ResponseWithError = Response & { error?: string }

/**
* Formalized way to define a route on a router:
* - Requires the caller to specify a JSON return-type for the endpoint,
Expand All @@ -61,12 +64,8 @@ export function defineRoute<TEndpoint extends keyof Routes>(
const handler: AnyRouterMiddleware = async (ctx, next) => {
let parsedBody: Record<string, unknown> = {}

try {
if (config.method !== 'get') {
parsedBody = await ctx.request.body({ type: 'json' }).value
}
// deno-lint-ignore no-empty
} catch {
if (config.method !== 'get') {
parsedBody = await ctx.request.body({ type: 'json' }).value
}

// if this route requires auth, decode the JWT payload and assert that
Expand Down
69 changes: 32 additions & 37 deletions back-end/routes/v1/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,43 +142,38 @@ export default function register(router: Router) {
handler: rateLimited(ONE_SECOND_MS, async ({ jwt, body: { invite_code } }) => {
const { account_id } = jwt

try {
return await withDBTransaction(async (db) => {
const inviteCodeResult = (await db.queryTable('invite_code', { where: ['code', '=', invite_code] }))[0]

if (inviteCodeResult == null) {
// invite code doesn't exist
return [null, Status.InternalServerError]
}

if (inviteCodeResult.used_by_account_id != null) {
// invite code already used
return [null, Status.InternalServerError]
}

const accountResult = await db.queryTable('account', { where: ['account_id', '=', account_id] })
const currentAccount = accountResult[0]
if (currentAccount == null) {
// account doesn't exist
return [null, Status.InternalServerError]
}

const codeUsedByCurrentAccount = (await db.queryTable('invite_code', { where: ['used_by_account_id', '=', account_id] }))[0]
if (codeUsedByCurrentAccount != null) {
// this account already used an invite code
return [null, Status.InternalServerError]
}

await db.updateTable('invite_code', {
used_by_account_id: account_id
}, [['code', '=', invite_code]])

return [null, Status.OK]
})
} catch (e) {
console.error(e)
return [null, Status.InternalServerError]
}
return await withDBTransaction(async (db) => {
const inviteCodeResult = (await db.queryTable('invite_code', { where: ['code', '=', invite_code] }))[0]

if (inviteCodeResult == null) {
// invite code doesn't exist
throw Error(`Invalid invite code submitted: "${invite_code}"`)
}

if (inviteCodeResult.used_by_account_id != null) {
// invite code already used
throw Error(`Already-used invite code submitted: "${invite_code}"`)
}

const accountResult = await db.queryTable('account', { where: ['account_id', '=', account_id] })
const currentAccount = accountResult[0]
if (currentAccount == null) {
// account doesn't exist
throw Error(`Tried to submit invite code from account that doesn't exist: "${account_id}"`)
}

const codeUsedByCurrentAccount = (await db.queryTable('invite_code', { where: ['used_by_account_id', '=', account_id] }))[0]
if (codeUsedByCurrentAccount != null) {
// this account already used an invite code
throw Error(`Account tried to submit an invite code but has already used one: "${account_id}"`)
}

await db.updateTable('invite_code', {
used_by_account_id: account_id
}, [['code', '=', invite_code]])

return [null, Status.OK]
})
}),
})
}
6 changes: 2 additions & 4 deletions back-end/routes/v1/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@ export default function register(router: Router) {
requireAuth: true,
handler: cached(3 * ONE_MINUTE_MS, async () => {
const festival = await withDBConnection(async db => (await db.queryTable('next_festival'))[0])

if (festival == null) {
return [null, Status.NotFound]
throw Error('Failed to find next_festival in the database')
}

const { festival_name, start_date, end_date } = festival

if (festival_name == null || start_date == null || end_date == null) {
return [null, Status.InternalServerError]
throw Error('next_festival incomplete')
}

return [{
Expand Down
5 changes: 2 additions & 3 deletions back-end/routes/v1/purchase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ export default function register(router: Router) {
(await db.queryTable('next_festival'))[0])

if (nextFestival == null) {
console.error('Failed to find next_festival in the database!')
return [null, Status.InternalServerError]
throw Error('Failed to find next_festival in the database')
}

const { allowedToPurchase } = await withDBConnection(db => accountReferralStatus(db, account_id, nextFestival.festival_id))
Expand Down Expand Up @@ -145,7 +144,7 @@ export default function register(router: Router) {
})
} break;
default:
console.warn(`Unhandled Stripe event type ${event.type}`);
console.warn(`\tUnhandled Stripe event type ${event.type}`);
}
})
}
9 changes: 4 additions & 5 deletions back-end/utils/mailgun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ const MAILGUN_DOMAIN = 'mail.vibe.camp'
const FROM = `Vibecamp <support@${MAILGUN_DOMAIN}>`

export type Email = {
to: string,
subject: string,
html: string,
readonly to: string,
readonly subject: string,
readonly html: string,
}

export async function sendMail(email: Email) {
Expand All @@ -30,8 +30,7 @@ export async function sendMail(email: Email) {
)

if (!res.ok) {
console.error('Failed to send mailgun email', res)
throw Error('Failed to send mailgun email')
throw Error(`Failed to send mailgun email: ${JSON.stringify(email)}`)
}
}

Expand Down
24 changes: 23 additions & 1 deletion back-end/utils/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,26 @@ export function exists<T>(val: T | null | undefined): val is T {

export function sum(a: number, b: number) {
return a + b
}
}

// Overengineering this a little to reduce allocations...
const spacesStrCache = new Map<number, string>()
export const spaces = (length: number): string => {
const cached = spacesStrCache.get(length)

if (cached != null) {
return cached
} else {
const str = new Array(length).fill(' ').join('')
spacesStrCache.set(length, str)
return str
}
}

export const pad = (str: string, length: number) => {
const spacesToAdd = Math.max(length - str.length, 0)
return str + spaces(spacesToAdd)
}

export const indent = (str: string) =>
str.split('\n').map(line => '\t' + line).join('\n')
3 changes: 2 additions & 1 deletion front-end/src/components/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ export default observer(() => {
return
}

const { response: { jwt }, status } = await vibefetch(null, `/${state.mode}`, 'post', {
const { response, status } = await vibefetch(null, `/${state.mode}`, 'post', {
email_address: loginForm.fields.emailAddress.value,
password: loginForm.fields.password.value
})
const { jwt } = response ?? {}

if (state.mode === 'login' && status === 401) {
throw 'Incorrect email or password'
Expand Down
3 changes: 2 additions & 1 deletion front-end/src/components/Tickets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ export default observer(() => {

const stripeOptions = useRequest(async () => {
if (Object.values(purchaseState.purchases).some(count => count > 0)) {
const { response: { stripe_client_secret } } = await vibefetch(
const { response } = await vibefetch(
Store.jwt,
'/purchase/create-intent',
'post',
purchaseState.purchases
)
const { stripe_client_secret } = response ?? {}

if (stripe_client_secret == null) {
return undefined
Expand Down
2 changes: 1 addition & 1 deletion front-end/src/vibefetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Routes } from '../../back-end/types/route-types'
import env from './env'
export const API_PREFIX = '/api/v1'

export async function vibefetch<TEndpoint extends keyof Routes>(jwt: string | null, input: TEndpoint, method: Routes[TEndpoint]['method'], body: Routes[TEndpoint]['body']): Promise<{ response: Routes[TEndpoint]['response'], status: number }> {
export async function vibefetch<TEndpoint extends keyof Routes>(jwt: string | null, input: TEndpoint, method: Routes[TEndpoint]['method'], body: Routes[TEndpoint]['body']) {
const res = await fetch(env.BACK_END_ORIGIN + API_PREFIX + input, {
method,
headers: {
Expand Down

0 comments on commit 3a9c2be

Please sign in to comment.