Skip to content

Commit

Permalink
[8.x] Swaps template literals for sprintf style interpolation (elasti…
Browse files Browse the repository at this point in the history
…c#200634) (elastic#200737)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Swaps template literals for sprintf style interpolation
(elastic#200634)](elastic#200634)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jason
Rhodes","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-19T14:02:09Z","message":"Swaps
template literals for sprintf style interpolation (elastic#200634)\n\n##
Summary\r\n\r\nString handling in error log output is now managed by
console.error\r\ninternals, for maximum safety re: external
input.\r\n\r\n## Comparison\r\n\r\n* The result of `formatErrors` is
being passed to `toString()` so that\r\nthe array stringification
matches the previous version. Otherwise, it\r\nwould include the `[ ]`
brackets around the array in the new log output.\r\n* The `apiUrl` and
`response` are still passed to `console.error` as the\r\nfinal two
arguments, so they'll continue to be printed the same way
as\r\nbefore.\r\n\r\nPrevious output (using a local jest test):\r\n<img
width=\"1495\" alt=\"Screenshot 2024-11-18 at 1 24
11 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/af08a471-ed94-4730-b5c7-fffbb3c9c9f9\">\r\n\r\nUpdated
output:\r\n<img width=\"1496\" alt=\"Screenshot 2024-11-18 at 1 46
34 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/7523bc5f-b367-4fb8-8dbb-e16c8f4fd43f\">\r\n\r\n\r\nThe
local jest test I used to confirm this was this:\r\n\r\n```ts\r\nimport
{ HttpSetup } from '@kbn/core-http-browser';\r\nimport { apiService }
from './api_service';\r\nimport * as rt from
'io-ts';\r\n\r\ndescribe('API service', () => {\r\n\r\n it('should log
the right error', async () => {\r\n const mockHttp = { \r\n fetch:
jest.fn(async () => ({\r\n data: {\r\n myKey: 'myvalue'\r\n }\r\n }))
\r\n } as unknown as HttpSetup;\r\n\r\n apiService.http =
mockHttp;\r\n\r\n const ResponseType = rt.type({\r\n data: rt.string\r\n
});\r\n\r\n await apiService.get(\r\n '/my/api/path', \r\n {}, \r\n
ResponseType\r\n );\r\n });\r\n\r\n});\r\n```\r\nand it was
in\r\n`x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts`\r\n\r\nTo
run, I used\r\n\r\n```sh\r\nnode scripts/jest
./x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts\r\n```\r\n\r\nThe
test always passes but it allows you to quickly/easily see the\r\noutput
when the condition fails.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"0c66148fc8480fa3fe844d0f304745dc4b62b946","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management"],"title":"Swaps
template literals for sprintf style
interpolation","number":200634,"url":"https://github.com/elastic/kibana/pull/200634","mergeCommit":{"message":"Swaps
template literals for sprintf style interpolation (elastic#200634)\n\n##
Summary\r\n\r\nString handling in error log output is now managed by
console.error\r\ninternals, for maximum safety re: external
input.\r\n\r\n## Comparison\r\n\r\n* The result of `formatErrors` is
being passed to `toString()` so that\r\nthe array stringification
matches the previous version. Otherwise, it\r\nwould include the `[ ]`
brackets around the array in the new log output.\r\n* The `apiUrl` and
`response` are still passed to `console.error` as the\r\nfinal two
arguments, so they'll continue to be printed the same way
as\r\nbefore.\r\n\r\nPrevious output (using a local jest test):\r\n<img
width=\"1495\" alt=\"Screenshot 2024-11-18 at 1 24
11 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/af08a471-ed94-4730-b5c7-fffbb3c9c9f9\">\r\n\r\nUpdated
output:\r\n<img width=\"1496\" alt=\"Screenshot 2024-11-18 at 1 46
34 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/7523bc5f-b367-4fb8-8dbb-e16c8f4fd43f\">\r\n\r\n\r\nThe
local jest test I used to confirm this was this:\r\n\r\n```ts\r\nimport
{ HttpSetup } from '@kbn/core-http-browser';\r\nimport { apiService }
from './api_service';\r\nimport * as rt from
'io-ts';\r\n\r\ndescribe('API service', () => {\r\n\r\n it('should log
the right error', async () => {\r\n const mockHttp = { \r\n fetch:
jest.fn(async () => ({\r\n data: {\r\n myKey: 'myvalue'\r\n }\r\n }))
\r\n } as unknown as HttpSetup;\r\n\r\n apiService.http =
mockHttp;\r\n\r\n const ResponseType = rt.type({\r\n data: rt.string\r\n
});\r\n\r\n await apiService.get(\r\n '/my/api/path', \r\n {}, \r\n
ResponseType\r\n );\r\n });\r\n\r\n});\r\n```\r\nand it was
in\r\n`x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts`\r\n\r\nTo
run, I used\r\n\r\n```sh\r\nnode scripts/jest
./x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts\r\n```\r\n\r\nThe
test always passes but it allows you to quickly/easily see the\r\noutput
when the condition fails.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"0c66148fc8480fa3fe844d0f304745dc4b62b946"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200634","number":200634,"mergeCommit":{"message":"Swaps
template literals for sprintf style interpolation (elastic#200634)\n\n##
Summary\r\n\r\nString handling in error log output is now managed by
console.error\r\ninternals, for maximum safety re: external
input.\r\n\r\n## Comparison\r\n\r\n* The result of `formatErrors` is
being passed to `toString()` so that\r\nthe array stringification
matches the previous version. Otherwise, it\r\nwould include the `[ ]`
brackets around the array in the new log output.\r\n* The `apiUrl` and
`response` are still passed to `console.error` as the\r\nfinal two
arguments, so they'll continue to be printed the same way
as\r\nbefore.\r\n\r\nPrevious output (using a local jest test):\r\n<img
width=\"1495\" alt=\"Screenshot 2024-11-18 at 1 24
11 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/af08a471-ed94-4730-b5c7-fffbb3c9c9f9\">\r\n\r\nUpdated
output:\r\n<img width=\"1496\" alt=\"Screenshot 2024-11-18 at 1 46
34 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/7523bc5f-b367-4fb8-8dbb-e16c8f4fd43f\">\r\n\r\n\r\nThe
local jest test I used to confirm this was this:\r\n\r\n```ts\r\nimport
{ HttpSetup } from '@kbn/core-http-browser';\r\nimport { apiService }
from './api_service';\r\nimport * as rt from
'io-ts';\r\n\r\ndescribe('API service', () => {\r\n\r\n it('should log
the right error', async () => {\r\n const mockHttp = { \r\n fetch:
jest.fn(async () => ({\r\n data: {\r\n myKey: 'myvalue'\r\n }\r\n }))
\r\n } as unknown as HttpSetup;\r\n\r\n apiService.http =
mockHttp;\r\n\r\n const ResponseType = rt.type({\r\n data: rt.string\r\n
});\r\n\r\n await apiService.get(\r\n '/my/api/path', \r\n {}, \r\n
ResponseType\r\n );\r\n });\r\n\r\n});\r\n```\r\nand it was
in\r\n`x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts`\r\n\r\nTo
run, I used\r\n\r\n```sh\r\nnode scripts/jest
./x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts\r\n```\r\n\r\nThe
test always passes but it allows you to quickly/easily see the\r\noutput
when the condition fails.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"0c66148fc8480fa3fe844d0f304745dc4b62b946"}}]}]
BACKPORT-->

Co-authored-by: Jason Rhodes <[email protected]>
  • Loading branch information
kibanamachine and jasonrhodes authored Nov 19, 2024
1 parent 74d2099 commit f38870d
Showing 1 changed file with 7 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,15 @@ class ApiService {
if (isRight(decoded)) {
return decoded.right as T;
} else {
// This was changed from using template literals to using %s string
// interpolation, but the previous version included the apiUrl value
// twice. To ensure the log output doesn't change, this continues.
//
// eslint-disable-next-line no-console
console.error(
`API ${apiUrl} is not returning expected response, ${formatErrors(
decoded.left
)} for response`,
'API %s is not returning expected response, %s for response',
apiUrl,
formatErrors(decoded.left).toString(),
apiUrl,
response
);
Expand Down

0 comments on commit f38870d

Please sign in to comment.