Allow OnErrorCallbacks to return Promise<boolean>

[imjoehaines]

Goal

Currently an async OnErrorCallback can only return Promise<void> in the Typescript, which means they can't be used to prevent events from sending without causing a compile error. Returning/resolving a boolean value does work, so we can safely allow Promise<boolean> as a return type

[github-actions]

@bugsnag/browser bundle size diff

	Minified	Minfied + Gzipped
Before	40.70 kB	12.54 kB
After	40.70 kB	12.54 kB
±	No change	No change

Generated by 🚫 dangerJS against 2ad9e21

[bengourley]

Changes look good, but since we're in this area I checked the node-style callback for correctness too and found it was also incorrect.

It's hard to get the typing strict enough that it doesn't let people write invalid sync/async without preventing people from writing valid node-style callbacks. I had a quick go and got close, but the never in this example makes it unusable for non-trivial node-style callbacks…

[imjoehaines]

From some more testing, it looks like we can't make it perfectly strict because of how void works; I've pushed a fix for node style callbacks and split out each type so it's easier to see the three valid signatures nevermind, this breaks inference of event

Because of void you can still do 'invalid' things like returning an array, but we ignore anything that's not false so this shouldn't actually be a problem

[bengourley]

Node-style callback signature is still not good. Is this the best we can do given the limitations?

[imjoehaines]

Node-style callback signature is still not good. Is this the best we can do given the limitations?

The alternatives are to either make it a union type, which makes event any:

type OnErrorCallback
  = ((event: Event) => void | boolean)
  | ((event: Event) => Promise<void | boolean>)
  | ((event: Event, cb: (err: null | Error, shouldSend?: boolean) => void) => void)

notify(event => event.stuff) // error TS7006: Parameter 'event' implicitly has an 'any' type.

or mark cb as always present, which isn't true but does seem to fix the other problems:

type OnErrorCallback = (event: Event, cb: (err: null | Error, shouldSend?: boolean) => void) => void | boolean | Promise<void | boolean>

notify(event => event.stuff) // works
notify((event, cb) => { cb(null) }) // works

notify(() => 1) // error TS2322: Type 'number' is not assignable to type 'boolean | void | Promise<boolean | void>'.

[bengourley]

mark cb as always present, which isn't true but does seem to fix the other problems:

I'd go with this. Marking it as always present doesn't mean people have to define it.