-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
e2e test for background error telemetry #8717
Changes from 1 commit
10cbeb1
3212d25
8f36be1
89beeef
15b06ea
311482c
9454929
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -26,54 +26,68 @@ async function waitForBackgroundPageRequest( | |||
return offscreenPage?.waitForRequest(errorServiceEndpoint); | ||||
} | ||||
|
||||
async function getSentErrors( | ||||
extensionId: string, | ||||
context: BrowserContext, | ||||
errorServiceEndpoint: "https://browser-intake-datadoghq.com/api/v2/*", | ||||
) { | ||||
// TODO: due to Datadog SDK implementation, it will take ~30 seconds for the | ||||
// request to be sent. We should figure out a way to induce the request to be sent sooner. | ||||
// See this datadog support request: https://help.datadoghq.com/hc/en-us/requests/1754158 | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤️ thanks again to reaching out to support |
||||
const request = await waitForBackgroundPageRequest( | ||||
context, | ||||
extensionId, | ||||
errorServiceEndpoint, | ||||
); | ||||
|
||||
return request | ||||
?.postData() | ||||
?.split("\n") | ||||
.map((log) => JSON.parse(log)); | ||||
} | ||||
|
||||
test.use({ | ||||
additionalRequiredEnvVariables: [ | ||||
"DATADOG_CLIENT_TOKEN", | ||||
"DEV_EVENT_TELEMETRY", | ||||
], | ||||
}); | ||||
|
||||
test("can report application error to telemetry service", async ({ | ||||
test("can report extension console error to telemetry service", async ({ | ||||
fungairino marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
page, | ||||
context, | ||||
extensionId, | ||||
}) => { | ||||
const errorServiceEndpoint = "https://browser-intake-datadoghq.com/api/v2/*"; | ||||
|
||||
await context.route( | ||||
"https://app.pixiebrix.com/api/extensions/", | ||||
async (route) => { | ||||
await route.fulfill({ | ||||
status: 200, | ||||
// Returning a bad response to trigger an error | ||||
body: JSON.stringify([{}]), | ||||
}); | ||||
}, | ||||
); | ||||
await test.step("Mock the extensions endpoint to return a bad response, and mock errorService calls", async () => { | ||||
await context.route( | ||||
"https://app.pixiebrix.com/api/extensions/", | ||||
async (route) => { | ||||
await route.fulfill({ | ||||
status: 200, | ||||
body: JSON.stringify([{}]), | ||||
}); | ||||
}, | ||||
); | ||||
|
||||
await context.route(errorServiceEndpoint, async (route) => | ||||
route.fulfill({ | ||||
status: 202, | ||||
}), | ||||
); | ||||
await context.route(errorServiceEndpoint, async (route) => | ||||
route.fulfill({ | ||||
status: 202, | ||||
}), | ||||
); | ||||
}); | ||||
|
||||
await page.goto(getBaseExtensionConsoleUrl(extensionId)); | ||||
await expect(page.getByText("Something went wrong.")).toBeVisible(); | ||||
|
||||
// TODO: due to Datadog SDK implementation, it will take ~30 seconds for the | ||||
// request to be sent. We should figure out a way to induce the request to be sent sooner. | ||||
const request = await waitForBackgroundPageRequest( | ||||
context, | ||||
const sentErrors = await getSentErrors( | ||||
extensionId, | ||||
context, | ||||
errorServiceEndpoint, | ||||
); | ||||
|
||||
const errorLogsJson = request | ||||
?.postData() | ||||
?.split("\n") | ||||
.map((log) => JSON.parse(log)); | ||||
|
||||
expect(errorLogsJson).toContainEqual( | ||||
expect(sentErrors).toContainEqual( | ||||
expect.objectContaining({ | ||||
service: "pixiebrix-browser-extension", | ||||
manifestVersion: 3, | ||||
|
@@ -82,6 +96,100 @@ test("can report application error to telemetry service", async ({ | |||
message: expect.any(String), | ||||
kind: expect.any(String), | ||||
}), | ||||
// Stack and message are duplicated | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what the intended goal with this one is. Is this a bug? Does I could at least see a comment like this being better suited for where we add the error message in the first place as opposed to a test, e.g. pixiebrix-extension/src/telemetry/logging.ts Line 430 in 3b6d29c
But at best I'd like to clarify if this is a problem or not, and create a ticket if so. |
||||
stack: expect.any(String), | ||||
message: expect.any(String), | ||||
connectionType: expect.any(String), | ||||
date: expect.any(Number), | ||||
extensionVersion: expect.any(String), | ||||
name: expect.any(String), | ||||
origin: "logger", | ||||
pageName: "options", | ||||
referrer: "", | ||||
runtimeId: extensionId, | ||||
session_id: expect.any(String), | ||||
status: "error", | ||||
url: `chrome-extension://${extensionId}/options.html#/`, | ||||
usr: { | ||||
email: "[email protected]", | ||||
id: "3f7ac0b4-5029-442c-b537-5de9f1dfdfd9", | ||||
organizationId: "47f616c5-81e3-4edb-ba44-ed5dd4a78c08", | ||||
}, | ||||
view: { | ||||
referrer: "", | ||||
url: `chrome-extension://${extensionId}/offscreen.html`, | ||||
}, | ||||
}), | ||||
); | ||||
}); | ||||
|
||||
test("can report a service worker error to telemetry service", async ({ | ||||
page, | ||||
context, | ||||
extensionId, | ||||
}) => { | ||||
const errorServiceEndpoint = "https://browser-intake-datadoghq.com/api/v2/*"; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: opportunity to extract errorServiceEndpoint |
||||
|
||||
await test.step("Mock the registry endpoint to return a bad response, and mock errorService calls", async () => { | ||||
await context.route( | ||||
"https://app.pixiebrix.com/api/registry/bricks/", | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would extract this endpoint into a constant called something like |
||||
async (route) => { | ||||
await route.fulfill({ | ||||
status: 500, | ||||
body: "I'm not json!", | ||||
}); | ||||
}, | ||||
); | ||||
|
||||
await context.route(errorServiceEndpoint, async (route) => | ||||
route.fulfill({ | ||||
status: 202, | ||||
}), | ||||
); | ||||
}); | ||||
|
||||
await page.goto(getBaseExtensionConsoleUrl(extensionId)); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be too early to extract at 2x usages, but maybe if we end up adding a third test with this shape, we should extract this logic into something reusable. |
||||
await expect(page.getByText("An error occurred")).toBeVisible(); | ||||
|
||||
const sentErrors = await getSentErrors( | ||||
extensionId, | ||||
context, | ||||
errorServiceEndpoint, | ||||
); | ||||
|
||||
expect(sentErrors).toContainEqual( | ||||
expect.objectContaining({ | ||||
code: "ERR_BAD_RESPONSE", | ||||
code_version: expect.any(String), | ||||
connectionType: "4g", | ||||
date: expect.any(Number), | ||||
error: expect.objectContaining({ | ||||
kind: "AxiosError", | ||||
message: expect.any(String), | ||||
stack: expect.any(String), | ||||
}), | ||||
extensionVersion: expect.any(String), | ||||
manifestVersion: 3, | ||||
message: expect.any(String), | ||||
name: "AxiosError", | ||||
origin: "logger", | ||||
pageName: "background", | ||||
referrer: "", | ||||
runtimeId: extensionId, | ||||
service: "pixiebrix-browser-extension", | ||||
session_id: expect.any(String), | ||||
stack: expect.any(String), | ||||
status: "error", | ||||
url: "https://app.pixiebrix.com/api/registry/bricks/", | ||||
usr: { | ||||
email: "[email protected]", | ||||
id: "3f7ac0b4-5029-442c-b537-5de9f1dfdfd9", | ||||
organizationId: "47f616c5-81e3-4edb-ba44-ed5dd4a78c08", | ||||
}, | ||||
view: { | ||||
referrer: "", | ||||
url: `chrome-extension://${extensionId}/offscreen.html`, | ||||
}, | ||||
}), | ||||
); | ||||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or
I don't know about anyone else, but my brain lumps "getSent" together instead of "sentErrors" together