SCAL-334630 stamp severity on SDK-raised EmbedEvent.Error - #642
SCAL-334630 stamp severity on SDK-raised EmbedEvent.Error#642sastaachar wants to merge 3 commits into
Conversation
Errors raised inside the embedded app already carry a severity, but errors the SDK raises itself never did, so a host could not apply one rule to both sources and had to sniff the error shape instead. - add EmbedErrorSeverity (SEV1/SEV2/SEV3), mirroring the levels the embedded app already uses, and expose it on EmbedErrorDetailsEvent - default to SEV3 in handleError so no error is emitted without one - SEV1 where nothing renders: missing init, failed login, missing Liveboard ID - SEV2 for the offline warning, SEV3 for API-intercept body parsing
There was a problem hiding this comment.
Code Review
This pull request introduces error severity levels (EmbedErrorSeverity) to the SDK, allowing hosts to determine how severely an error impacts the embedded application. It updates various error dispatch locations to include a severity level (defaulting to SEV3) and adds corresponding unit tests. Feedback on the changes suggests ensuring that console logs capture the defaulted severity rather than the raw error details, and correcting a minor style guide violation in the JSDoc comments where a complete sentence lacks a trailing period.
| * @group Error Handling | ||
| * | ||
| * @example | ||
| * Only surface errors that took the embed down |
There was a problem hiding this comment.
The description for the @example tag is a complete sentence but does not end with a period. According to the style guide, every description that forms a complete sentence should end with a period.
| * Only surface errors that took the embed down | |
| * Only surface errors that took the embed down. |
References
- Every description that forms a complete sentence should end with a period. (link)
The parity spec requires every symbol exported from src/index.ts to be re-exported from src/react/all-types-export.ts so React consumers can import it. EmbedErrorSeverity was missing, failing the build.
commit: |
Summary
Errors raised inside the embedded ThoughtSpot app already carry a
severity(the app stamps one on every
EmbedEvent.Errorit dispatches, defaulting toSEV3). Errors the SDK itself raises never did. A host that wants a single
rule for "is this worth showing the user?" therefore had to sniff the error
shape —
typeof error.error === 'string'— to tell SDK-raised errors apartfrom app-raised ones.
This adds severity to the SDK side so one rule covers both.
Changes
EmbedErrorSeverityenum (SEV1/SEV2/SEV3), mirroring the levelsthe embedded app already uses, exported from the package root.
EmbedErrorDetailsEvent.severity— optional, so existing callers areunaffected.
handleErrordefaults to SEV3, so nothing is emitted without a severity.A raising site that sets its own severity wins.
INIT_SDK_REQUIRED,LOGIN_FAILED,LIVEBOARD_VIZ_ID_VALIDATION.API-intercept body parsing.
No behaviour changes for anyone not reading
severity; the field is additiveand optional.
Testing
ts-embed,liveboard,api-interceptdefault, and that an explicit severity is not overridden
npm run check-size— 31.8 kB against the 34 kB limitAdditional Notes
Motivated by the playground templates in
scaligent, which now gate the errorbanner on
severity === 1. Until this lands, SDK-raised errors carry noseverity and so never trip that gate.