Skip to content

Commit

Permalink
Merge pull request #7264 from TerriaJS/error-service-refactor
Browse files Browse the repository at this point in the history
Refactor error service
  • Loading branch information
ljowen authored Sep 19, 2024
2 parents 2dc8739 + e07d908 commit a21ecf7
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 107 deletions.
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

#### next release (8.7.7)

- **Breaking changes:**

- Remove RollbarErrorServiceProvder
- Error services now instantiated externally to terriajs

- [The next improvement]

#### 8.7.6 - 2024-08-22
Expand Down
8 changes: 4 additions & 4 deletions doc/customizing/client-side-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**

Expand Down
27 changes: 5 additions & 22 deletions lib/Models/ErrorServiceProviders/ErrorService.ts
Original file line number Diff line number Diff line change
@@ -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<ErrorServiceProvider> {
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;
}
39 changes: 0 additions & 39 deletions lib/Models/ErrorServiceProviders/RollbarErrorServiceProvider.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
}
37 changes: 15 additions & 22 deletions lib/Models/Terria.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) => ({
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
72 changes: 53 additions & 19 deletions test/Models/ErrorServiceSpec.ts
Original file line number Diff line number Diff line change
@@ -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));
});
});

0 comments on commit a21ecf7

Please sign in to comment.