feat(loadtesting): added sas worker file #36929
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a service worker file (sasworker.js) to enable proper rendering of Playwright traces in the Azure portal. The service worker intercepts requests to Azure Blob Storage URLs, extracts trace file references with SAS tokens, and redirects them to the official Playwright trace viewer.
Key Changes
- Added a new service worker (sasworker.js) that intercepts fetch requests to Azure Blob Storage URLs and redirects trace viewers to trace.playwright.dev with properly encoded trace URLs and SAS parameters
- Modified the build script to include a copy-static step that distributes the service worker to all output directories (commonjs, esm, browser, react-native)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| sdk/loadtesting/playwright/src/utils/sasworker.js | New service worker implementing URL interception and redirection logic for Playwright trace viewing |
| sdk/loadtesting/playwright/package.json | Updated build script to include copy-static step for distributing the service worker file to all distribution directories |
Comments suppressed due to low confidence (1)
sdk/loadtesting/playwright/package.json:60
- The check-format and format scripts in package.json only target TypeScript files but not JavaScript files in the src directory. Since sasworker.js is a source file that should follow the project's formatting standards, the pattern should include JavaScript files. Consider updating the patterns from "src//*.ts" to "src//*.{ts,js}" in both scripts.
"check-format": "prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"samples/**/*.ts\" \"*.{js,json}\"",
"clean": "rimraf --glob dist *.log dist-test temp types *.tgz *.xml samples/**/test-results/",
"execute:samples": "echo skipped",
"extract-api": "dev-tool run extract-api",
"format": "prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"samples/**/*.ts\" \"*.{js,json}\"",
| /* ============================================================ | ||
| Service Worker: Redirect trace/index.html to trace.playwright.dev | ||
| - Intercepts navigation to trace/index.html with trace parameter | ||
| - Extracts and properly encodes the trace URL | ||
| - Redirects to trace.playwright.dev viewer | ||
| ============================================================ */ | ||
|
|
||
| self.addEventListener("install", () => self.skipWaiting()); | ||
| self.addEventListener("activate", (event) => event.waitUntil(self.clients.claim())); | ||
|
|
||
| self.addEventListener("fetch", (event) => { | ||
| const url = new URL(event.request.url); | ||
|
|
||
| // Only Azure Blob | ||
| if (!url.hostname.endsWith(".blob.core.windows.net")) return; | ||
|
|
||
| // Never intercept the worker itself | ||
| if (url.pathname.endsWith("/sasworker.js")) return; | ||
|
|
||
| // Intercept trace/index.html with trace parameter | ||
| if (url.pathname.includes("/trace/index.html") && url.searchParams.has("trace")) { | ||
|
|
||
| // Extract the trace parameter value (URL-encoded blob URL) | ||
| const traceParam = url.searchParams.get("trace"); | ||
|
|
||
| // Decode it to get the actual blob URL | ||
| const decodedTraceUrl = decodeURIComponent(traceParam); | ||
|
|
||
| // The SAS params are separate query params on the main URL, not part of trace param | ||
| // We need to append them to the decoded trace URL | ||
| const traceUrl = new URL(decodedTraceUrl); | ||
|
|
||
| // Copy all SAS params from the main URL to the trace URL | ||
| for (const [key, value] of url.searchParams.entries()) { | ||
| if (key !== "trace") { | ||
| traceUrl.searchParams.set(key, value); | ||
| } | ||
| } | ||
|
|
||
| const completeTraceUrl = traceUrl.toString(); | ||
|
|
||
| // Now encode the entire URL (base + query params together) | ||
| const properlyEncodedUrl = encodeURIComponent(completeTraceUrl); | ||
|
|
||
| // Build the trace.playwright.dev URL | ||
| const traceViewerUrl = `https://trace.playwright.dev/?trace=${properlyEncodedUrl}`; | ||
|
|
||
| event.respondWith(Response.redirect(traceViewerUrl, 302)); | ||
| return; | ||
| } | ||
| }); |
There was a problem hiding this comment.
The sasworker.js file introduces new functionality for redirecting trace URLs, but there are no corresponding tests for this behavior. Other utility files in this package have comprehensive test coverage (e.g., utils.spec.ts, packageManager.spec.ts). Consider adding tests to verify the service worker's URL interception, parameter extraction, encoding, and redirection logic, especially for edge cases like malformed URLs or missing parameters.
| "scripts": { | ||
| "build": "npm run clean && dev-tool run build-package && dev-tool run extract-api", | ||
| "build": "npm run clean && dev-tool run build-package && npm run copy-static && dev-tool run extract-api", | ||
| "copy-static": "node -e \"const fs = require('fs'); const path = require('path'); const src = 'src/utils/sasworker.js'; const targets = ['dist/commonjs/utils/sasworker.js', 'dist/esm/utils/sasworker.js', 'dist/browser/utils/sasworker.js', 'dist/react-native/utils/sasworker.js']; targets.forEach(target => { fs.mkdirSync(path.dirname(target), {recursive: true}); fs.copyFileSync(src, target); });\"", |
There was a problem hiding this comment.
The copy-static script is implemented as an inline Node.js script which makes it difficult to read and maintain. Consider extracting this logic into a separate build script file (e.g., scripts/copy-static.js) for better maintainability, error handling, and testability. This would also make it easier to add logging or handle edge cases.
| "build": "npm run clean && dev-tool run build-package && npm run copy-static && dev-tool run extract-api", | ||
| "copy-static": "node -e \"const fs = require('fs'); const path = require('path'); const src = 'src/utils/sasworker.js'; const targets = ['dist/commonjs/utils/sasworker.js', 'dist/esm/utils/sasworker.js', 'dist/browser/utils/sasworker.js', 'dist/react-native/utils/sasworker.js']; targets.forEach(target => { fs.mkdirSync(path.dirname(target), {recursive: true}); fs.copyFileSync(src, target); });\"", |
There was a problem hiding this comment.
The sasworker.js file is copied to all distribution directories but is not exported from the package's main entry point (index.ts) and is not included in the package.json exports field. If this service worker is intended to be accessible to package consumers, consider adding an export path in the package.json exports field (e.g., "./utils/sasworker.js") so users can import or reference it properly. If it's only for internal use or deployment, add a comment documenting the intended usage pattern.
| // Build the trace.playwright.dev URL | ||
| const traceViewerUrl = `https://trace.playwright.dev/?trace=${properlyEncodedUrl}`; |
There was a problem hiding this comment.
The service worker redirects to an external domain (trace.playwright.dev) without any validation. While this appears intentional for Playwright trace viewing, consider documenting this behavior clearly and ensuring it aligns with security policies. If trace.playwright.dev is the official Playwright domain, this should be documented. Additionally, consider adding a comment explaining why this external redirect is safe and expected.
|
|
||
| // The SAS params are separate query params on the main URL, not part of trace param | ||
| // We need to append them to the decoded trace URL | ||
| const traceUrl = new URL(decodedTraceUrl); |
There was a problem hiding this comment.
The URL constructor on line 31 can throw a TypeError if the decoded trace parameter is not a valid URL. This error would bubble up unhandled in the service worker. Consider wrapping this logic in a try-catch block to handle malformed URLs gracefully and prevent the service worker from failing.
| const traceUrl = new URL(decodedTraceUrl); | |
| let traceUrl; | |
| try { | |
| traceUrl = new URL(decodedTraceUrl); | |
| } catch (e) { | |
| // If the decoded trace parameter is not a valid URL, do not intercept; | |
| // allow the request to proceed normally to avoid failing the service worker. | |
| // Optional: log for diagnostics without disrupting normal operation. | |
| // eslint-disable-next-line no-console | |
| console.warn("sasworker: invalid trace URL in 'trace' query parameter", e); | |
| return; | |
| } |
3 participants
Packages impacted by this PR
@azure/playwright
Issues associated with this PR
sas worker file was needed to render trace in portal.
Describe the problem that is addressed by this PR
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists