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.

Sign up for GitHub

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

Set result status code when returning a non-200 response #412

Merged
merged 3 commits into from
Aug 25, 2021
Merged

Set result status code when returning a non-200 response #412

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16bb3c4
Set the response status code when there are errors.
rocketnova Aug 24, 2021
58db8e2
Fix tests.
rocketnova Aug 24, 2021
5da0028
Merge branch 'main' into rocket/406-non-200-response
rocketnova Aug 25, 2021
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
7 changes: 6 additions & 1 deletion pages/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default function Home({
)
}

export const getServerSideProps: GetServerSideProps = async ({ req, locale, query }) => {
export const getServerSideProps: GetServerSideProps = async ({ req, res, locale, query }) => {
const isProd = process.env.NODE_ENV === 'production'
const logger = isProd ? pino({}) : pino({ prettyPrint: true })
logger.info(req)
Expand Down Expand Up @@ -123,6 +123,11 @@ export const getServerSideProps: GetServerSideProps = async ({ req, locale, quer
// that in our links back out to UIO.
const userArrivedFromUioMobile = query?.from === 'uiom'

// If there is an errorCode, set the response statusCode to match.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// If there is an errorCode, set the response statusCode to match.
// If there is an errorCode from the API gateway, set our app's response statusCode to match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kalvinwang I'm not including this suggestion because the error code is set for issues that are not limited to errors from the API gateway. It's any application level error (e.g. env vars missing, catching a thrown error, etc).

Copy link
Contributor

Choose a reason for hiding this comment

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

ooh, got it, thanks!

if (errorCode) {
res.statusCode = errorCode
}

// Return Props.
return {
props: {
Expand Down
4 changes: 4 additions & 0 deletions tests/pages/indexServerSide.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@ describe('Main component server side logic', () => {
// empty header
headers: {},
},
res: {
statusCode: null,
},
}

const result: GetServerSideProps = await getServerSideProps(context as GetServerSidePropsContext)
const props: HomeProps = result.props as HomeProps
expect(props.errorCode).toBe(500)
expect(context.res.statusCode).toBe(500)

restore()
})
Expand Down