-
Notifications
You must be signed in to change notification settings - Fork 57
feat: Create logging service #1137
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
Changes from all commits
732d2f0
fc4681b
7ff3c41
28b8892
0b3dc81
c095cbd
6fe5d08
7e41e15
2b829ce
9934b81
1d1ebfa
82a8d4a
c93096d
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 | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,192 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { ErrorCodes, LogRequestBody, WSDKErrorSeverity } from "./types"; | ||||||||||||||||||||||||||||||||||||||||||
| import { FetchUploader, IFetchPayload } from "../uploaders"; | ||||||||||||||||||||||||||||||||||||||||||
| import { IStore, SDKConfig } from "../store"; | ||||||||||||||||||||||||||||||||||||||||||
| import { SDKInitConfig } from "../sdkRuntimeModels"; | ||||||||||||||||||||||||||||||||||||||||||
| import Constants from "../constants"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Header key constants | ||||||||||||||||||||||||||||||||||||||||||
| const HEADER_ACCEPT = 'Accept' as const; | ||||||||||||||||||||||||||||||||||||||||||
| const HEADER_CONTENT_TYPE = 'Content-Type' as const; | ||||||||||||||||||||||||||||||||||||||||||
| const HEADER_ROKT_LAUNCHER_VERSION = 'rokt-launcher-version' as const; | ||||||||||||||||||||||||||||||||||||||||||
| const HEADER_ROKT_LAUNCHER_INSTANCE_GUID = 'rokt-launcher-instance-guid' as const; | ||||||||||||||||||||||||||||||||||||||||||
| const HEADER_ROKT_WSDK_VERSION = 'rokt-wsdk-version' as const; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| interface IReportingLoggerPayload extends IFetchPayload { | ||||||||||||||||||||||||||||||||||||||||||
| headers: IFetchPayload['headers'] & { | ||||||||||||||||||||||||||||||||||||||||||
| [HEADER_ROKT_LAUNCHER_INSTANCE_GUID]?: string; | ||||||||||||||||||||||||||||||||||||||||||
| [HEADER_ROKT_LAUNCHER_VERSION]: string; | ||||||||||||||||||||||||||||||||||||||||||
| [HEADER_ROKT_WSDK_VERSION]: string; | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
| body: string; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| /** | |
| * Logger responsible for reporting Web SDK logs and errors to the configured backend services. | |
| * | |
| * This class builds and sends log payloads (such as info and error events) using the configured | |
| * logging and error endpoints, SDK/store configuration, and an optional launcher instance GUID. | |
| * It also applies rate limiting via an {@link IRateLimiter} implementation to avoid excessive | |
| * reporting and respects feature flags such as `isWebSdkLoggingEnabled`. | |
| */ |
Copilot
AI
Feb 13, 2026
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.
The config parameter is typed as 'SDKConfig | SDKInitConfig | any', where 'any' removes all type safety. This makes it impossible for TypeScript to catch type errors in config usage. Consider creating a union type or interface that includes all valid config properties, or at minimum use 'unknown' instead of 'any' to require explicit type checking.
Copilot
AI
Feb 13, 2026
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.
The isEnabled flag is calculated once in the constructor and never updated. If the feature flag (isWebSdkLoggingEnabled) changes after initialization, or if the debug mode query parameter is added/removed during the page lifecycle, the reporting behavior won't reflect these changes. Consider making isEnabled a getter that re-evaluates the condition on each access, or add a method to update the flag when configuration changes.
Copilot
AI
Feb 13, 2026
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.
Trailing whitespace on this line. Remove the trailing whitespace for consistency with code style.
Copilot
AI
Feb 13, 2026
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.
The error handling silently catches and logs all exceptions to console. In production, this could mask issues with the reporting service itself. Consider adding a flag or counter to track repeated failures, or consider not catching certain types of errors (like network timeouts) that might indicate a systemic issue that should be addressed.
Copilot
AI
Feb 13, 2026
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.
The getVersion() method returns either the integration name from store or a fallback string with SDK version. However, the header is named 'rokt-launcher-version' which suggests it should contain version information, not a name. If store.getIntegrationName() returns a name like 'custom-integration-name' (as shown in test line 197), this won't be a version. Consider either renaming the header to 'rokt-launcher-name' or ensuring the value is actually a version string.
| return this.store?.getIntegrationName?.() ?? `mParticle_wsdkv_${this.sdkVersion}`; | |
| return `mParticle_wsdkv_${this.sdkVersion}`; |
Copilot
AI
Feb 13, 2026
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.
Trailing whitespace on this line. Remove the trailing whitespace for consistency with code style.
Copilot
AI
Feb 13, 2026
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.
The optional chaining across multiple lines (116-120) is fragile and hard to read. Consider simplifying this check by extracting the query parameter check into a separate helper function, or by checking for window.location first and then checking the search parameter. This would make the code more maintainable and testable.
| return ( | |
| typeof window !== 'undefined' && | |
| (window. | |
| location?. | |
| search?. | |
| toLowerCase()?. | |
| includes('mp_enable_logging=true') ?? false) | |
| ); | |
| } | |
| return this.isDebugModeEnabledViaQueryParam(); | |
| } | |
| private isDebugModeEnabledViaQueryParam(): boolean { | |
| if (typeof window === 'undefined' || !window.location || !window.location.search) { | |
| return false; | |
| } | |
| const search = window.location.search.toLowerCase(); | |
| return search.includes('mp_enable_logging=true'); | |
| } |
Copilot
AI
Feb 13, 2026
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.
Trailing whitespace on this line. Remove the trailing whitespace for consistency with code style.
| const newCount = count + 1; | |
| this.logCount.set(severity, newCount); | |
| const newCount = count + 1; | |
| this.logCount.set(severity, newCount); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { valueof } from '../utils'; | ||
|
|
||
| export const ErrorCodes = { | ||
| UNKNOWN_ERROR: 'UNKNOWN_ERROR', | ||
| UNHANDLED_EXCEPTION: 'UNHANDLED_EXCEPTION', | ||
| IDENTITY_REQUEST: 'IDENTITY_REQUEST', | ||
| } as const; | ||
|
|
||
| export type ErrorCodes = valueof<typeof ErrorCodes>; | ||
|
|
||
| export type ErrorCode = ErrorCodes | string; | ||
|
alexs-mparticle marked this conversation as resolved.
|
||
|
|
||
| export const WSDKErrorSeverity = { | ||
| ERROR: 'ERROR', | ||
| INFO: 'INFO', | ||
| WARNING: 'WARNING', | ||
| } as const; | ||
|
|
||
| export type WSDKErrorSeverity = (typeof WSDKErrorSeverity)[keyof typeof WSDKErrorSeverity]; | ||
|
|
||
| export type ErrorsRequestBody = { | ||
| additionalInformation?: Record<string, string>; | ||
| code: ErrorCode; | ||
| severity: WSDKErrorSeverity; | ||
| stackTrace?: string; | ||
| deviceInfo?: string; | ||
| integration?: string; | ||
| reporter?: string; | ||
| url?: string; | ||
| }; | ||
|
|
||
| export type LogRequestBody = ErrorsRequestBody; | ||
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.
The string concatenation here uses both the ' + ' operator and string concatenation with quotes. For consistency and readability, consider using template literals instead:
Error sending identity request to servers - ${errorMessage}