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.

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

feature: add custom http status code with error object #1334

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 81 additions & 3 deletions __tests__/servers/web/web.js
Expand Up @@ -686,15 +686,41 @@ describe('Server: Web', () => {
name: 'statusTestAction',
description: 'I am a test',
inputs: {
key: { required: true }
key: { required: true },
query: { required: false },
randomKey: { required: false }
},
run: (data) => {
if (data.params.key !== 'value') {
data.connection.rawConnection.responseHttpCode = 402
throw new Error('key != value')
} else {
data.response.good = true
}
const hasQueryParam = !!data.params.query
if (hasQueryParam) {
const validQueryFilters = ['test', 'search']
const validQueryParam = validQueryFilters.indexOf(data.params.query) > -1
if (!validQueryParam) {
const notFoundError = new Error(`404: Filter '${data.params.query}' not found `)
notFoundError.code = 404
throw notFoundError
}
}
const hasRandomKey = !!data.params.randomKey
Copy link
Member

Choose a reason for hiding this comment

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

I've been out of touch with JS for a bit. What does !!data.params.randomkey do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If randomKey is not passed in the request, so it will be undefined, so !undefined will be true and doing !!undefined will become false which means that the variable is not sent.

If randomKey is set it will return true for !randomKey and doing !!randomKey will return true.

I wanted to not do this kind of thing as I know it seems confusing, but I was unable to find any kind of isEmpty sort-of Util but was unable to find, so used this type of thing.

Should I remove this and create an isEmpty sort-of Util and go ahead with that?

Copy link
Member

Choose a reason for hiding this comment

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

I believe the typical convention is to use const hasRandomKey = data.params && data.params.randomKey. Has that changed recently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely right, doing const hasRandomKey = data.params && data.params.randomKey would produce the same result, but then hasRandomKey will contain the value of data.params.randomKey not a boolean, I thought that since the variable name is hasRandomKey it represents a boolean, plus that makes easy to read the if condition rather than predicting what it will do. Is this a good way to go?

if (hasRandomKey) {
const validRandomKeys = ['key1', 'key2', 'key3']
const validRandomKey = validRandomKeys.indexOf(data.params.randomKey) > -1
if (!validRandomKey) {
if (data.params.randomKey === 'expired-key') {
const expiredError = new Error(`999: Key '${data.params.randomKey}' is expired`)
expiredError.code = 999
throw expiredError
}
const suspiciousError = new Error(`402: Suspicious Activity detected with key ${data.params.randomKey}`)
suspiciousError.code = 402
throw suspiciousError
}
}
data.response.good = true
}
}
}
Expand Down Expand Up @@ -743,6 +769,58 @@ describe('Server: Web', () => {
const body = await toJson(response.body)
expect(body.good).toEqual(true)
})
describe('setting status code using custom errors', () => {
test('should work for 404 status code, set using custom error for invalid params', async () => {
try {
await request.post(url + '/api/statusTestAction', { form: { key: 'value', query: 'guess' } })
throw new Error('should not get here')
} catch (error) {
expect(error.statusCode).toEqual(404)
const body = await toJson(error.response.body)
expect(body.error).toEqual('404: Filter \'guess\' not found ')
}
})

test('should work for 402 status code set using custom error for invalid params', async () => {
try {
await request.post(url + '/api/statusTestAction', { form: { key: 'value', randomKey: 'guessKey' } })
throw new Error('should not get here')
} catch (error) {
expect(error.statusCode).toEqual(402)
const body = await toJson(error.response.body)
expect(body.error).toEqual('402: Suspicious Activity detected with key guessKey')
}
})

test('should not throw custom error for valid params', async () => {
const responseWithQuery = await request.post(url + '/api/statusTestAction', { form: { key: 'value', query: 'test' }, resolveWithFullResponse: true })
expect(responseWithQuery.statusCode).toEqual(200)
const responseBody = await toJson(responseWithQuery.body)
expect(responseBody.good).toEqual(true)

const responseWithRandomKey = await request.post(url + '/api/statusTestAction', { form: { key: 'value', randomKey: 'key1' }, resolveWithFullResponse: true })
expect(responseWithRandomKey.statusCode).toEqual(200)
const body = await toJson(responseWithRandomKey.body)
expect(body.good).toEqual(true)

const responseWithKeyAndQuery = await request.post(url + '/api/statusTestAction', { form: { key: 'value', query: 'search', randomKey: 'key2' }, resolveWithFullResponse: true })
expect(responseWithKeyAndQuery.statusCode).toEqual(200)
const receivedBody = await toJson(responseWithKeyAndQuery.body)
expect(receivedBody.good).toEqual(true)
})

test('should not work for 999 status code set using custom error and default error code, 400 is thrown', async () => {
try {
await request.post(url + '/api/statusTestAction', { form: { key: 'value', randomKey: 'expired-key' } })
throw new Error('should not get here')
} catch (error) {
expect(error.statusCode).not.toEqual(999)
expect(error.statusCode).toEqual(400)
evantahler marked this conversation as resolved.
Show resolved Hide resolved
const body = await toJson(error.response.body)
expect(body.error).toEqual('999: Key \'expired-key\' is expired')
}
})
})
})

describe('documentation', () => {
Expand Down
6 changes: 5 additions & 1 deletion servers/web.js
Expand Up @@ -348,7 +348,11 @@ module.exports = class WebServer extends ActionHero.Server {

if (data.response.error) {
if (this.config.returnErrorCodes === true && data.connection.rawConnection.responseHttpCode === 200) {
if (data.actionStatus === 'unknown_action') {
const customErrorCode = parseInt(data.response.error.code, 10)
const isValidCustomResponseCode = customErrorCode >= 100 && customErrorCode < 600
evantahler marked this conversation as resolved.
Show resolved Hide resolved
if (isValidCustomResponseCode) {
data.connection.rawConnection.responseHttpCode = customErrorCode
} else if (data.actionStatus === 'unknown_action') {
data.connection.rawConnection.responseHttpCode = 404
} else if (data.actionStatus === 'missing_params') {
data.connection.rawConnection.responseHttpCode = 422
Expand Down
25 changes: 25 additions & 0 deletions tutorials/web-server.md
Expand Up @@ -151,6 +151,31 @@ exports['default'] = {
},
// When true, returnErrorCodes will modify the response header for http(s) clients if connection.error is not null.
// You can also set connection.rawConnection.responseHttpCode to specify a code per request.
/**
* To create custom Error objects with custom response code you need to add the your response status code in the error object.
* // Note: Error code in error object will overwrite the response's status code.
* @example
*
* // Your action
* {
* ...
* const raiseNotFound = <your-condition>
* if(raiseNotFound) {
* const notFoundError = new Error(<msg>)
* notFoundError.code = 404
* throw notFoundError
* }
* ...
* const raiseSuspiciousActivity = <your-condition>
* if(raiseSuspiciousActivity) {
* const suspiciousActivity = new Error(<msg>)
* suspiciousActivity.code = 402
* throw suspiciousActivity
* }
* ...
* }
* // Only the values between 100 and 599 are accepted for status code, otherwise, it will be ignored.
* */
returnErrorCodes: true,
// should this node server attempt to gzip responses if the client can accept them?
// this will slow down the performance of actionhero, and if you need this funcionality, it is recommended that you do this upstream with nginx or your load balancer
Expand Down