Skip to content

Commit

Permalink
[HTTP] Add more info to the log.error metadata (#169552)
Browse files Browse the repository at this point in the history
  • Loading branch information
@afharo
afharo committed Oct 24, 2023
1 parent 0a25b39 commit 47ade5c
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 12 deletions.
33 changes: 23 additions & 10 deletions packages/core/http/core-http-router-server-internal/src/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,26 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo

public handleLegacyErrors = wrapErrors;

private logError(
msg: string,
statusCode: number,
{
error,
request,
}: {
request: Request;
error: Error;
}
) {
this.log.error(msg, {
http: {
response: { status_code: statusCode },
request: { method: request.route?.method, path: request.route?.path },
},
error: { message: error.message },
});
}

private async handle<P, Q, B>({
routeSchemas,
request,
Expand All @@ -200,10 +220,7 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
try {
kibanaRequest = CoreKibanaRequest.from(request, routeSchemas);
} catch (error) {
this.log.error(`400 Bad Request`, {
http: { response: { status_code: 400 } },
});

this.logError('400 Bad Request', 400, { request, error });
return hapiResponseAdapter.toBadRequest(error.message);
}

Expand All @@ -216,18 +233,14 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo

// forward 401 errors from ES client
if (isElasticsearchUnauthorizedError(error)) {
this.log.error(`401 Unauthorized`, {
http: { response: { status_code: 401 } },
});
this.logError('401 Unauthorized', 401, { request, error });
return hapiResponseAdapter.handle(
kibanaResponseFactory.unauthorized(convertEsUnauthorized(error))
);
}

// return a generic 500 to avoid error info / stack trace surfacing
this.log.error(`500 Server Error`, {
http: { response: { status_code: 500 } },
});
this.logError('500 Server Error', 500, { request, error });
return hapiResponseAdapter.toInternalError();
}
}
Expand Down
74 changes: 72 additions & 2 deletions src/core/server/integration_tests/http/router.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,12 @@ describe('Handler', () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');

router.get({ path: '/', validate: false }, (context, req, res) => {
router.get({ path: '/{query}', validate: false }, (context, req, res) => {
throw Boom.unauthorized();
});
await server.start();

const result = await supertest(innerServer.listener).get('/').expect(500);
const result = await supertest(innerServer.listener).get('/some-data').expect(500);

expect(result.body.message).toBe(
'An internal server error occurred. Check Kibana server logs for details.'
Expand All @@ -616,7 +616,14 @@ describe('Handler', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "Unauthorized",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/{query}",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -645,7 +652,14 @@ describe('Handler', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "Unexpected result from Route Handler. Expected KibanaResponse, but given: string.",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -689,7 +703,14 @@ describe('Handler', () => {
Array [
"400 Bad Request",
Object {
"error": Object {
"message": "[request query.page]: expected value of type [number] but got [string]",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 400,
},
Expand Down Expand Up @@ -1173,7 +1194,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "expected 'location' header to be set",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -1586,7 +1614,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "Unexpected Http status code. Expected from 400 to 599, but given: 200",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -1662,7 +1697,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "expected 'location' header to be set",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -1809,7 +1851,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "expected error message to be provided",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -1842,7 +1891,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "expected error message to be provided",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -1874,7 +1930,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "options.statusCode is expected to be set. given options: undefined",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down Expand Up @@ -1906,7 +1969,14 @@ describe('Response factory', () => {
Array [
"500 Server Error",
Object {
"error": Object {
"message": "Unexpected Http status code. Expected from 100 to 599, but given: 20.",
},
"http": Object {
"request": Object {
"method": "get",
"path": "/",
},
"response": Object {
"status_code": 500,
},
Expand Down

0 comments on commit 47ade5c

Please sign in to comment.