From ee5e1ac795f4d410de3a4246d92a3303c177a67d Mon Sep 17 00:00:00 2001 From: Lawrence Owen Date: Tue, 13 Aug 2024 10:54:16 +1000 Subject: [PATCH 1/3] Breaking change to ErrorService: - remove rollbar service - ErrorService now passed in as a startOption, configuration passed to init function called on the service. --- CHANGES.md | 4 ++ doc/customizing/client-side-config.md | 8 +-- .../ErrorServiceProviders/ErrorService.ts | 27 ++----- .../RollbarErrorServiceProvider.ts | 39 ---------- .../StubErrorServiceProvider.ts | 1 + lib/Models/Terria.ts | 37 ++++------ package.json | 1 - test/Models/ErrorServiceSpec.ts | 72 ++++++++++++++----- 8 files changed, 82 insertions(+), 107 deletions(-) delete mode 100644 lib/Models/ErrorServiceProviders/RollbarErrorServiceProvider.ts diff --git a/CHANGES.md b/CHANGES.md index 0f7d555feb7..25a9fd647f6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ #### next release (8.7.6) +- **Breaking changes:** +- Remove RollbarErrorServiceProvder +- Error services now instantiated externally to terriajs + - Set default value for date and datetime WPS fields only when the field is marked as required. - [The next improvement] diff --git a/doc/customizing/client-side-config.md b/doc/customizing/client-side-config.md index 908d4f82a7a..342eecd61f7 100644 --- a/doc/customizing/client-side-config.md +++ b/doc/customizing/client-side-config.md @@ -180,10 +180,10 @@ Configuration of items to appear in the search bar ### ErrorServiceOptions -| Name | Required | Type | Default | Description | -| ------------- | -------- | ---------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| provider | yes | **string** | `undefined` | A string identifying the error service provider to use. Currently only `rollbar` is supported. | -| configuration | no | **any** | `undefined` | The configuration object to pass as constructor parameters to the error service provider instance. See the [provider implementation](https://github.com/TerriaJS/terriajs/blob/main/lib/Models/ErrorServiceProviders/) for supported configuration parameters. | +| Name | Required | Type | Default | Description | +| ------------- | -------- | ---------- | ----------- | ---------------------------------------------------------------------- | +| provider | no | **string** | `undefined` | A string identifying the error service provider. | +| configuration | no | **any** | `undefined` | The configuration object to be used when initializing an ErrorService. | **Example** diff --git a/lib/Models/ErrorServiceProviders/ErrorService.ts b/lib/Models/ErrorServiceProviders/ErrorService.ts index 03c59b904e6..dd506e3b773 100644 --- a/lib/Models/ErrorServiceProviders/ErrorService.ts +++ b/lib/Models/ErrorServiceProviders/ErrorService.ts @@ -1,31 +1,14 @@ import TerriaError from "../../Core/TerriaError"; - +import { ConfigParameters } from "../Terria"; export interface ErrorServiceOptions { - provider: string; + provider?: string; configuration: any; } /** * The interface that all valid error service providers must implement. */ -export type ErrorServiceProvider = { - error(error: string | Error | TerriaError): void; -}; - -/** - * Asynchronously loads and returns an error service provider instance for the given options. - */ -export async function initializeErrorServiceProvider( - options?: ErrorServiceOptions -): Promise { - const provider = options?.provider; - const configuration = options?.configuration; - - if (provider === "rollbar") { - const rollbarModule = await import("./RollbarErrorServiceProvider"); - const rollbarProvider = new rollbarModule.default(configuration); - return rollbarProvider; - } else { - throw new Error(`Unknown error service provider: ${provider}`); - } +export interface ErrorServiceProvider { + init: (config: ConfigParameters) => void; + error: (error: string | Error | TerriaError) => void; } diff --git a/lib/Models/ErrorServiceProviders/RollbarErrorServiceProvider.ts b/lib/Models/ErrorServiceProviders/RollbarErrorServiceProvider.ts deleted file mode 100644 index c2831b6a57d..00000000000 --- a/lib/Models/ErrorServiceProviders/RollbarErrorServiceProvider.ts +++ /dev/null @@ -1,39 +0,0 @@ -import Rollbar from "rollbar"; -import TerriaError, { TerriaErrorSeverity } from "../../Core/TerriaError"; -import { ErrorServiceProvider } from "./ErrorService"; - -export default class RollbarErrorServiceProvider - implements ErrorServiceProvider -{ - readonly rollbar: Rollbar; - - /** - * @param configuration Configuration for the Rollbar instance. See https://docs.rollbar.com/docs/rollbarjs-configuration-reference#context-1 - * - * Caveat: Rollbar API requests are blocked by some privacy extensions for browsers. - */ - constructor(configuration: any) { - this.rollbar = new Rollbar({ - captureUncaught: true, - captureUnhandledRejections: true, - enabled: true, - ...configuration - }); - } - - error(error: string | Error | TerriaError) { - if (error instanceof TerriaError) { - if ( - (typeof error.severity === "function" - ? error.severity() - : error.severity) === TerriaErrorSeverity.Error - ) { - this.rollbar.error(error.toError()); - } else { - this.rollbar.warning(error.toError()); - } - } else { - this.rollbar.error(error); - } - } -} diff --git a/lib/Models/ErrorServiceProviders/StubErrorServiceProvider.ts b/lib/Models/ErrorServiceProviders/StubErrorServiceProvider.ts index 689d8280992..6ac53fe3eb4 100644 --- a/lib/Models/ErrorServiceProviders/StubErrorServiceProvider.ts +++ b/lib/Models/ErrorServiceProviders/StubErrorServiceProvider.ts @@ -5,5 +5,6 @@ import { ErrorServiceProvider } from "./ErrorService"; * A stub error service provider that does nothing. */ export default class StubErrorServiceProvider implements ErrorServiceProvider { + init() {} error(_error: string | Error | TerriaError) {} } diff --git a/lib/Models/Terria.ts b/lib/Models/Terria.ts index ea1e1b2fdb5..c31d61a7ead 100644 --- a/lib/Models/Terria.ts +++ b/lib/Models/Terria.ts @@ -96,8 +96,7 @@ import updateModelFromJson from "./Definition/updateModelFromJson"; import upsertModelFromJson from "./Definition/upsertModelFromJson"; import { ErrorServiceOptions, - ErrorServiceProvider, - initializeErrorServiceProvider + ErrorServiceProvider } from "./ErrorServiceProviders/ErrorService"; import StubErrorServiceProvider from "./ErrorServiceProviders/StubErrorServiceProvider"; import TerriaFeature from "./Feature/Feature"; @@ -360,6 +359,7 @@ interface StartOptions { }; applicationUrl?: Location; shareDataService?: ShareDataService; + errorService?: ErrorServiceProvider; /** * i18nOptions is explicitly a separate option from `languageConfiguration`, * as `languageConfiguration` can be serialised, but `i18nOptions` may have @@ -679,8 +679,8 @@ export default class Terria { readonly developmentEnv = process?.env?.NODE_ENV === "development"; /** - * An error service instance. The instance can be configured by setting the - * `errorService` config parameter. Here we initialize it to stub provider so + * An error service instance. The instance can be provided via the + * `errorService` startOption. Here we initialize it to stub provider so * that the `terria.errorService` always exists. */ errorService: ErrorServiceProvider = new StubErrorServiceProvider(); @@ -912,19 +912,6 @@ export default class Terria { this.modelIdShareKeysMap.set(id, [shareKey]); } - /** - * Initialize errorService from config parameters. - */ - setupErrorServiceProvider(errorService: ErrorServiceOptions) { - initializeErrorServiceProvider(errorService) - .then((errorService) => { - this.errorService = errorService; - }) - .catch((e) => { - console.error("Failed to initialize error service", e); - }); - } - setupInitializationUrls(baseUri: uri.URI, config: any) { const initializationUrls: string[] = config?.initializationUrls || []; const initSources: InitSource[] = initializationUrls.map((url) => ({ @@ -1016,10 +1003,6 @@ export default class Terria { if (isJsonObject(config) && isJsonObject(config.parameters)) { this.updateParameters(config.parameters); } - - if (this.configParameters.errorService) { - this.setupErrorServiceProvider(this.configParameters.errorService); - } this.setupInitializationUrls(baseUri, config); }); } catch (error) { @@ -1041,7 +1024,17 @@ export default class Terria { setCustomRequestSchedulerDomainLimits( this.configParameters.customRequestSchedulerLimits ); - + if (options.errorService) { + try { + this.errorService = options.errorService; + this.errorService.init(this.configParameters); + } catch (e) { + console.error( + `Failed to initialize error service: ${this.configParameters.errorService?.provider}`, + e + ); + } + } this.analytics?.start(this.configParameters); this.analytics?.logEvent( Category.launch, diff --git a/package.json b/package.json index 8ad64208205..09da43ea118 100644 --- a/package.json +++ b/package.json @@ -166,7 +166,6 @@ "react-virtual": "~2.3.2", "resolve-url-loader": "^5.0.0", "retry": "^0.12.0", - "rollbar": "^2.24.0", "sass-loader": "^10", "shpjs": "^4.0.4", "simple-statistics": "^7.0.1", diff --git a/test/Models/ErrorServiceSpec.ts b/test/Models/ErrorServiceSpec.ts index eaa5ca9fa7c..9361386623f 100644 --- a/test/Models/ErrorServiceSpec.ts +++ b/test/Models/ErrorServiceSpec.ts @@ -1,25 +1,59 @@ -import { initializeErrorServiceProvider } from "../../lib/Models/ErrorServiceProviders/ErrorService"; -import RollbarErrorServiceProvider from "../../lib/Models/ErrorServiceProviders/RollbarErrorServiceProvider"; +import TerriaError from "../../lib/Core/TerriaError"; +import { ErrorServiceProvider } from "../../lib/Models/ErrorServiceProviders/ErrorService"; +import StubErrorServiceProvider from "../../lib/Models/ErrorServiceProviders/StubErrorServiceProvider"; +import Terria from "../../lib/Models/Terria"; -describe("initializeErrorServiceProvider", function () { - it("can initialize RollbarErrorServiceProvider", async function () { - const errorService = await initializeErrorServiceProvider({ - provider: "rollbar", - configuration: {} +describe("ErrorService", function () { + let mockErrorServiceProvider: ErrorServiceProvider; + let terria: Terria; + + beforeAll(() => { + jasmine.Ajax.stubRequest(/.*/).andError({}); + jasmine.Ajax.stubRequest(/.*(serverconfig|proxyabledomains).*/).andReturn({ + responseText: JSON.stringify({ foo: "bar" }) + }); + jasmine.Ajax.stubRequest("test-config.json").andReturn({ + responseText: JSON.stringify({ config: true }) }); - expect(errorService instanceof RollbarErrorServiceProvider).toBe(true); + mockErrorServiceProvider = { + init: () => {}, + error: () => {} + }; }); - it("throws an error when an invalid provider type is given", async function () { - let error; - try { - await initializeErrorServiceProvider({ - provider: "foo", - configuration: undefined - }); - } catch (e: any) { - error = e; - } - expect(error.message).toBe(`Unknown error service provider: foo`); + beforeEach(() => { + terria = new Terria({ + appBaseHref: "/", + baseUrl: "./" + }); + }); + + it("Initializes an error service, passing in config", async function () { + const initSpy = spyOn(mockErrorServiceProvider, "init"); + await terria.start({ + configUrl: "test-config.json", + errorService: mockErrorServiceProvider + }); + expect(initSpy).toHaveBeenCalledTimes(1); + }); + + it("Gets called with error", async function () { + const errorSpy = spyOn(mockErrorServiceProvider, "error").and.callThrough(); + await terria.start({ + configUrl: "test-config.json", + errorService: mockErrorServiceProvider + }); + const error = new TerriaError({ + message: "test error" + }); + terria.raiseErrorToUser(error); + expect(errorSpy).toHaveBeenCalledWith(error); + }); + + it("Falls back to stub provider", () => { + terria.start({ + configUrl: "test-config.json" + }); + expect(terria.errorService).toEqual(jasmine.any(StubErrorServiceProvider)); }); }); From 75a5abdc0f6c4fc019cf435b7bb91a5ea818de6e Mon Sep 17 00:00:00 2001 From: Lawrence Owen Date: Thu, 19 Sep 2024 10:33:54 +1000 Subject: [PATCH 2/3] Update CHANGES.md --- CHANGES.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1944e4cbfb4..d7d37b13c31 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,14 +2,12 @@ #### next release (8.7.7) -<<<<<<< error-service-refactor - **Breaking changes:** -- Remove RollbarErrorServiceProvder -- Error services now instantiated externally to terriajs + + - Remove RollbarErrorServiceProvder + - Error services now instantiated externally to terriajs - Set default value for date and datetime WPS fields only when the field is marked as required. -======= ->>>>>>> main - [The next improvement] #### 8.7.6 - 2024-08-22 From e07d90853b1c15082701c6c69636e18cd540e205 Mon Sep 17 00:00:00 2001 From: Lawrence Owen Date: Thu, 19 Sep 2024 14:19:40 +1000 Subject: [PATCH 3/3] Update CHANGES.md --- CHANGES.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index d7d37b13c31..0d746a9124a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,7 +7,6 @@ - Remove RollbarErrorServiceProvder - Error services now instantiated externally to terriajs -- Set default value for date and datetime WPS fields only when the field is marked as required. - [The next improvement] #### 8.7.6 - 2024-08-22