Skip to content

Commit

Permalink
Error handling improvements
Browse files Browse the repository at this point in the history
Proposing some changes that I mentioned [here](#236 (comment)). To recap, I think the error logging in Stimulus is great, but several people (me included) have been caught by the fact that errors aren't reported elsewhere out of the box. The aim of this PR is to automatically integrate with third party error tracking services where possible, and to improve the documentation where not.

Specific changes:

- If `window.onerror` is defined, Stimulus will now call it after logging an error. Many error tracking tools define this method, eg. [Sentry](https://github.com/getsentry/sentry-javascript/blob/0ee07995d415d3870608c477cbdcf8445a51e1bb/packages/browser/src/loader.js#L192), [Airbrake](https://github.com/airbrake/airbrake-js/blob/9d4787b1c559aa39107d7288f46c4108c9a9d954/packages/browser/src/notifier.ts#L70)
- Added documentation on how error handling works, including what happens out of the box and how to override it (with code sample from #53)
- Added tests for the error handler.
  • Loading branch information
ghiculescu committed Jan 8, 2021
1 parent c47f551 commit e37a2be
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 1 deletion.
20 changes: 20 additions & 0 deletions docs/handbook/07_installing_stimulus.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,29 @@ Stimulus supports all evergreen, self-updating desktop and mobile browsers out o
If your application needs to support older browsers like Internet Explorer 11, include the [`@stimulus/polyfills`](https://www.npmjs.com/package/@stimulus/polyfills) package before loading Stimulus.

```js
// src/application.js
import "@stimulus/polyfills"
import { Application } from "stimulus"

const application = Application.start()
//
```

## Error handling

All calls from Stimulus to your application's code are wrapped in a `try ... catch` block.

If your code throws an error, it will be caught by Stimulus and logged to the browser console, including extra detail such as the controller name and event or lifecycle function being called. If you use an error tracking system that defines `window.onerror`, Stimulus will also pass the error on to it.

You can override how Stimulus handles errors by defining `Application#handleError`:

```js
// src/application.js
import { Application } from "stimulus"
const application = Application.start()

application.handleError = (error, message, detail) => {
console.warn(message, detail)
Raven.captureException(error)
}
```
2 changes: 2 additions & 0 deletions packages/@stimulus/core/src/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export class Application implements ErrorHandler {

handleError(error: Error, message: string, detail: object) {
this.logger.error(`%s\n\n%o\n\n%o`, message, error, detail)

window.onerror && window.onerror(message, "", 0, 0, error)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ class TestApplication extends Application {
}
}

// should only be used in tests that are testing error handling behavior
// in all other tests, use `TestApplication` so all errors are raised (and cause tests to fail)
export class TestApplicationWithDefaultErrorBehavior extends Application {
}

export class ApplicationTestCase extends DOMTestCase {
schema: Schema = defaultSchema
application: Application = new TestApplication(this.fixtureElement, this.schema)
Expand Down
2 changes: 1 addition & 1 deletion packages/@stimulus/core/src/tests/modules/class_tests.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ControllerTestCase } from "../cases/controller_test_case"
import { ClassController } from "../controllers/class_controller"

export default class ValueTests extends ControllerTestCase(ClassController) {
export default class ClassTests extends ControllerTestCase(ClassController) {
fixtureHTML = `
<div data-controller="${this.identifier}"
data-${this.identifier}-active-class="test--active"
Expand Down
59 changes: 59 additions & 0 deletions packages/@stimulus/core/src/tests/modules/error_handler_tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { Controller } from "../.."
import { ControllerTestCase } from "../cases/controller_test_case"
import { TestApplicationWithDefaultErrorBehavior } from "../cases/application_test_case"

class MockLogger {
errors: any[] = []
logs: any[] = []
warns: any[] = []

log(event: any) {
this.logs.push(event)
}

error(event: any) {
this.errors.push(event)
}

warn(event: any) {
this.warns.push(event)
}
}

class ErrorWhileConnectingController extends Controller {
connect() {
throw new Error('bad!');
}
}

export default class ErrorHandlerTests extends ControllerTestCase(ErrorWhileConnectingController) {
controllerConstructor = ErrorWhileConnectingController

async setupApplication() {
const logger = new MockLogger()

this.application = new TestApplicationWithDefaultErrorBehavior(this.fixtureElement, this.schema)
this.application.logger = logger

window.onerror = function(message, source, lineno, colno, error) {
logger.log(`error from window.onerror. message = ${message}, source = ${source}, lineno = ${lineno}, colno = ${colno}`)
}

await super.setupApplication()
}

async "test errors in connect are thrown and handled by built in logger"() {
const mockLogger: any = this.application.logger

// when `ErrorWhileConnectingController#connect` throws, the controller's application's logger's `error` function
// is called; in this case that's `MockLogger#error`.
this.assert.equal(1, mockLogger.errors.length)
}

async "test errors in connect are thrown and handled by window.onerror"() {
const mockLogger: any = this.application.logger

this.assert.equal(1, mockLogger.logs.length)
this.assert.equal('error from window.onerror. message = Error connecting controller, source = , lineno = 0, colno = 0', mockLogger.logs[0])
}
}

0 comments on commit e37a2be

Please sign in to comment.