Return Boom errors directly to the browser for Time Series Visual Builder #11656
Conversation
simianhacker
added
Feature:Visualizations
| @@ -1,5 +1,5 @@ | |||
| export default panel => error => { | |||
| console.log(error); | |||
| if (error.isBoom) throw error; | |||
There was a problem hiding this comment.
I think it's probably ideal for errors of all types to use boom so that they follow the error path and only trigger .catch() handlers. In this case you can dictate the rendered version of the boom error like so:
const result = {};
// populate result
const err = Boom.create(...);
err.output.payload = result // assign the result as the output payload
throw err;There was a problem hiding this comment.
At this point we only want to throw the 401 to breakout of the promise chain so the UI can show the auth dialog box. I will change this to check for the status 401.
| @@ -7,7 +7,12 @@ export default (server) => { | |||
| handler: (req, reply) => { | |||
| return getFields(req) | |||
There was a problem hiding this comment.
hapi route handlers are not promise aware, but could be if you pass the promise to reply instead of returning it. Then the resolve value or error is treated as the argument to reply (like you are doing below).
| @@ -9,7 +9,7 @@ export default (server) => { | |||
| return getVisData(req) | |||
| .then(reply) | |||
| .catch(err => { | |||
| console.error(err.stack); | |||
| if (err.isBoom) return reply(err); | |||
There was a problem hiding this comment.
Boom.wrap() just returns the error if it is already a boom error, so this isn't necessary.
| @@ -7,7 +7,12 @@ export default (server) => { | |||
| handler: (req, reply) => { | |||
| return getFields(req) | |||
| .then(reply) | |||
| .catch(() => reply([])); | |||
| .catch((err) => { | |||
| if (err.isBoom) { | |||
There was a problem hiding this comment.
Is there a better condition that we could use here? The fact that the error is a Boom error doesn't really mean anything specific... Maybe a check for err.output.statusCode === 401?
|
@spalger I believe I addressed all your comments. |
epixa
changed the title
…lder (elastic#11656) * Fixed elastic#11643 - Return Boom errors directly to the browser * Checking for 401 and boom errors instead of just boom errors * removing the returns from the hapi routes
This PR fixes #11643 by returning Boom errors to the client directly instead of serializing them into the response. Errors from the ES client usually are Boom errors when there is an authentication issue.