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.

Sign up for GitHub

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

feat: add logging of latency to both error and success messages #27

Closed
wants to merge 1 commit into from
Closed

feat: add logging of latency to both error and success messages #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/http/fetcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,16 @@

// This is how "default tooling" prints errors
// eslint-disable-next-line @typescript-eslint/no-base-to-string
expect(String(err)).toMatchInlineSnapshot(`"HttpRequestError: 500 GET some"`)
expect(String(err)).toMatchInlineSnapshot(`"HttpRequestError: 500 GET some 93 ms"`)

Check failure on line 157 in src/http/fetcher.test.ts

View workflow job for this annotation

GitHub Actions / build-job

mocking fetch 

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `mocking fetch 2`

Snapshot: "HttpRequestError: 500 GET some 93 ms"
Received: "HttpRequestError: 500 GET some 70 ms"

    at Object.<anonymous> (src/http/fetcher.test.ts:157:23)

// This is how Jest prints errors
expect(err).toMatchInlineSnapshot('[HttpRequestError: 500 GET some]')
expect(err).toMatchInlineSnapshot(`[HttpRequestError: 500 GET some 93 ms]`)

Check failure on line 160 in src/http/fetcher.test.ts

View workflow job for this annotation

GitHub Actions / build-job

mocking fetch 

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `mocking fetch 3`

Snapshot: [HttpRequestError: 500 GET some 93 ms]
Received: [HttpRequestError: 500 GET some 70 ms]

    at Object.<anonymous> (src/http/fetcher.test.ts:160:15)

// This is how NC-ecosystem-aware consumer prints errors (e.g with Cause)
expect(_stringify(err)).toMatchInlineSnapshot(`

Check failure on line 163 in src/http/fetcher.test.ts

View workflow job for this annotation

GitHub Actions / build-job

mocking fetch 

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `mocking fetch 4`

- Snapshot  - 1
+ Received  + 1

- HttpRequestError: 500 GET some 93 ms
+ HttpRequestError: 500 GET some 70 ms
  Caused by: AppError: aya-baya

    at Object.<anonymous> (src/http/fetcher.test.ts:163:27)
"HttpRequestError: 500 GET some
Caused by: AppError: aya-baya"
`)
"HttpRequestError: 500 GET some 93 ms
Caused by: AppError: aya-baya"
`)
err.data.requestDuration = 10 // mock stability
expect(err.data).toMatchInlineSnapshot(`
{
Expand Down Expand Up @@ -248,7 +248,7 @@
url: 'some',
})
_assertIsError(err)
expect(String(err)).toMatchInlineSnapshot(`"HttpRequestError: GET some"`)
expect(String(err)).toMatchInlineSnapshot(`"HttpRequestError: GET some 5 ms"`)

Check failure on line 251 in src/http/fetcher.test.ts

View workflow job for this annotation

GitHub Actions / build-job

json parse error 

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `json parse error 1`

Snapshot: "HttpRequestError: GET some 5 ms"
Received: "HttpRequestError: GET some 3 ms"

    at Object.<anonymous> (src/http/fetcher.test.ts:251:23)
_assertIsErrorObject(err.cause)
delete err.cause.stack
expect(err.cause).toMatchInlineSnapshot(`
Expand All @@ -261,10 +261,10 @@
}
`)

expect(_stringify(err)).toMatchInlineSnapshot(`

Check failure on line 264 in src/http/fetcher.test.ts

View workflow job for this annotation

GitHub Actions / build-job

json parse error 

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `json parse error 3`

- Snapshot  - 1
+ Received  + 1

- HttpRequestError: GET some 5 ms
+ HttpRequestError: GET some 3 ms
  Caused by: JsonParseError: Failed to parse: some text

    at Object.<anonymous> (src/http/fetcher.test.ts:264:27)
"HttpRequestError: GET some
Caused by: JsonParseError: Failed to parse: some text"
`)
"HttpRequestError: GET some 5 ms
Caused by: JsonParseError: Failed to parse: some text"
`)
})

test('paginate', async () => {
Expand Down Expand Up @@ -383,9 +383,9 @@
expectTypeOf(err).toEqualTypeOf<HttpRequestError>()
expect(err.data.requestMethod).toBe('POST')
expect(_stringify(err)).toMatchInlineSnapshot(`
"HttpRequestError: 500 POST https://example.com/
Caused by: Error: bad"
`)
"HttpRequestError: 500 POST https://example.com/ 0 ms
Caused by: Error: bad"
`)
} else {
expectTypeOf(data).toEqualTypeOf<{ ok: boolean }>()
}
Expand Down Expand Up @@ -460,9 +460,9 @@

const err = await fetcher.expectError({ url: 'someUrl' })
expect(_stringify(err)).toMatchInlineSnapshot(`
"HttpRequestError: 500 GET someUrl
Caused by: AppError: some"
`)
"HttpRequestError: 500 GET someUrl 0 ms
Caused by: AppError: some"
`)

// 2. Pass should throw
jest
Expand Down
6 changes: 4 additions & 2 deletions src/http/fetcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ export class Fetcher {
' <<',
res.fetchResponse.status,
res.signature,
retryAttempt && `try#${retryAttempt + 1}/${req.retry.count + 1}`,
_since(res.req.started),
retryAttempt && `try#${retryAttempt + 1}/${req.retry.count + 1}`,
Copy link
Author

Choose a reason for hiding this comment

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

Change of order for consistency with error message and ease of parsing

]
.filter(Boolean)
.join(' '),
Expand Down Expand Up @@ -466,7 +466,9 @@ export class Fetcher {
responseStatusCode = 0
}

const message = [res.statusCode, res.signature].filter(Boolean).join(' ')
const message = [res.statusCode, res.signature, _since(res.req.started)]
.filter(Boolean)
.join(' ')

res.err = new HttpRequestError(
message,
Expand Down
Loading