Skip to content

Commit

Permalink
feat: opt-in for error reporting (#4279)
Browse files Browse the repository at this point in the history
fixes: #4272
  • Loading branch information
AlCalzone committed Mar 1, 2022
1 parent 0df7e20 commit d8d6233
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 30 deletions.
10 changes: 10 additions & 0 deletions docs/api/driver.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ The following table gives you an overview of what happens during the startup pro
| 4 | - | `"all nodes ready"` event is emitted for the driver when all nodes can be used |
| 5 | - | `"interview completed"` event is emitted for every node when its interview is completed for the first time. This only gets emitted once, unless the node gets re-interviewed. |

### `enableErrorReporting`

```ts
enableErrorReporting(): void
```

Enable sending crash reports using [Sentry](https://sentry.io). These reports are important for quickly discovering unhandled errors in the library, so we **we kindly ask you** to enable them. Please do **not** enable them in dev environments where frequent errors are to be expected.

> [!NOTE] Sentry registers a global `unhandledRejection` event handler, which has an influence how the application will behave in case of an unhandled rejection. This can affect applications on Node.js before `v15`, which are going to crash instead of logging a warning.
### `enableStatistics`

```ts
Expand Down
6 changes: 6 additions & 0 deletions docs/getting-started/migrating-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,9 @@ While there is no hard style guide governing this, we tend to use readonly prope
## Upgraded `serialport` library to version 10.x

The `serialport` library was upgraded to version 10.x. It is now built using N-API, which should avoid having to recompile it after upgrades. Due to slight API changes, overwriting the dependency to specific `9.x` versions is no longer possible.

## Error-reporting is now opt-in

It was found that the global `unhandledRejection` event handler which Sentry registers, has an influence how the application will behave in case of an unhandled rejection. This can affect applications on Node.js before `v15`, which are going to crash instead of logging a warning. While we do believe no rejection should be silently unhandled, we don't want to break existing applications.

Therefore, error reporting is now opt-in using `driver.enableErrorReporting()`. We kindly ask you to do so in production environments, so unhandled errors in the library can be discovered more quickly.
2 changes: 1 addition & 1 deletion packages/zwave-js/src/Driver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { Driver } from "./lib/driver/Driver";
export { Driver, libName, libVersion } from "./lib/driver/Driver";
export type { SendMessageOptions } from "./lib/driver/Driver";
export type { FileSystem } from "./lib/driver/FileSystem";
export type { ZWaveOptions } from "./lib/driver/ZWaveOptions";
Expand Down
23 changes: 0 additions & 23 deletions packages/zwave-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,6 @@ import "reflect-metadata";
import { install as installSourceMapSupport } from "source-map-support";
installSourceMapSupport();

import * as path from "path";
import { initSentry } from "./lib/telemetry/sentry";

/** The version of zwave-js, exported for your convenience */
const packageJsonPath = require.resolve("zwave-js/package.json");
// eslint-disable-next-line @typescript-eslint/no-var-requires
const packageJson = require(packageJsonPath);
const libraryRootDir = path.dirname(packageJsonPath);
const libName: string = packageJson.name;
const libVersion: string = packageJson.version;

// Init sentry, unless we're running a a test or some custom-built userland or PR test versions
if (
process.env.NODE_ENV !== "test" &&
!/\-[a-f0-9]{7,}$/.test(libVersion) &&
!/\-pr\-\d+\-$/.test(libVersion)
) {
void initSentry(libraryRootDir, libName, libVersion).catch(() => {
/* ignore */
});
}

// Export some frequently-used things and types - this also loads all CC files including metadata
export * from "./CommandClass";
export * from "./Controller";
Expand All @@ -38,4 +16,3 @@ export * from "./Error";
export * from "./Node";
export * from "./Utils";
export * from "./Values";
export { libVersion };
22 changes: 21 additions & 1 deletion packages/zwave-js/src/lib/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ import type { ZWaveNode } from "../node/Node";
import { InterviewStage, NodeStatus } from "../node/Types";
import type { SerialAPIStartedRequest } from "../serialapi/misc/SerialAPIStartedRequest";
import { reportMissingDeviceConfig } from "../telemetry/deviceConfig";
import { initSentry } from "../telemetry/sentry";
import {
AppInfo,
compileStatistics,
Expand Down Expand Up @@ -173,7 +174,8 @@ const packageJsonPath = require.resolve("zwave-js/package.json");
// eslint-disable-next-line @typescript-eslint/no-var-requires
const packageJson = require(packageJsonPath);
const libraryRootDir = path.dirname(packageJsonPath);
const libVersion: string = packageJson.version;
export const libVersion: string = packageJson.version;
export const libName: string = packageJson.name;

// This is made with cfonts:
const libNameString = `
Expand Down Expand Up @@ -1402,6 +1404,24 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks> {
});
}

/**
* Enables error reporting via Sentry. This is turned off by default, because it registers a
* global `unhandledRejection` event handler, which has an influence how the application will
* behave in case of an unhandled rejection.
*/
public enableErrorReporting(): void {
// Init sentry, unless we're running a a test or some custom-built userland or PR test versions
if (
process.env.NODE_ENV !== "test" &&
!/\-[a-f0-9]{7,}$/.test(libVersion) &&
!/\-pr\-\d+\-$/.test(libVersion)
) {
void initSentry(libraryRootDir, libName, libVersion).catch(() => {
/* ignore */
});
}
}

private _statisticsEnabled: boolean = false;
/** Whether reporting usage statistics is currently enabled */
public get statisticsEnabled(): boolean {
Expand Down
6 changes: 1 addition & 5 deletions test/run.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
// To test with Sentry reporting:
// import { Driver } from "../packages/zwave-js";

// To test without Sentry reporting
import path from "path";
import "reflect-metadata";
import { Driver } from "../packages/zwave-js/src/lib/driver/Driver";
import { Driver } from "../packages/zwave-js/src";

process.on("unhandledRejection", (_r) => {
debugger;
Expand Down

0 comments on commit d8d6233

Please sign in to comment.