Skip to content

Commit

Permalink
Swaps template literals for sprintf style interpolation (elastic#200634)
Browse files Browse the repository at this point in the history
## Summary

String handling in error log output is now managed by console.error
internals, for maximum safety re: external input.

## Comparison

* The result of `formatErrors` is being passed to `toString()` so that
the array stringification matches the previous version. Otherwise, it
would include the `[ ]` brackets around the array in the new log output.
* The `apiUrl` and `response` are still passed to `console.error` as the
final two arguments, so they'll continue to be printed the same way as
before.

Previous output (using a local jest test):
<img width="1495" alt="Screenshot 2024-11-18 at 1 24 11 PM"
src="https://github.com/user-attachments/assets/af08a471-ed94-4730-b5c7-fffbb3c9c9f9">

Updated output:
<img width="1496" alt="Screenshot 2024-11-18 at 1 46 34 PM"
src="https://github.com/user-attachments/assets/7523bc5f-b367-4fb8-8dbb-e16c8f4fd43f">


The local jest test I used to confirm this was this:

```ts
import { HttpSetup } from '@kbn/core-http-browser';
import { apiService } from './api_service';
import * as rt from 'io-ts';

describe('API service', () => {

  it('should log the right error', async () => {
    const mockHttp = { 
      fetch: jest.fn(async () => ({
        data: {
          myKey: 'myvalue'
        }
      })) 
    } as unknown as HttpSetup;

    apiService.http = mockHttp;

    const ResponseType = rt.type({
      data: rt.string
    });

    await apiService.get(
      '/my/api/path', 
      {}, 
      ResponseType
    );
  });

});
```
and it was in
`x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts`

To run, I used

```sh
node scripts/jest ./x-pack/plugins/observability_solution/synthetics/public/utils/api_service/api_service.test.ts
```

The test always passes but it allows you to quickly/easily see the
output when the condition fails.

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
2 people authored and paulinashakirova committed Nov 26, 2024
1 parent f8358fa commit 47d0e81
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 47d0e81

Please sign in to comment.