Add callback argument to sendError and wrap (#514)

Currently the `sendError` functions (and in turn the `wrap` helper) has
`tags` and `namespace` parameters. We've seen in the past that this is
quite limiting for the `sendError` helper as users can't add additional
metadata like an action name, parameters, etc.

Deprecate the `tags` and `namespace` parameters in favor of the new
callback function. The deprecations will log a warning to the console
when it's detected they are used. I've not disabled their functionality
so users can upgrade at their own pace.

I'm not very familiar with TypeScript definitions so I've done my best
with what I saw in the rest of the library and found in the docs. The
`tags` parameter in the `send` and `sendError` functions now serves the
double purpose of `tags` parameter and callback function. I've done my
best to differentiate between these two function signatures by defining
multiple signatures.

The thing that was most unclear to me was passing the multi purpose
parameter as a HashMap to the `setTags` function. I ended up casting it
to a HashMap with the `toHashMap` function I saw used elsewhere, only
here I also needed to cast it explicitly to a `HashMap`, hence the
import.

Part of appsignal/integration-guide#41
We also need to update the docs.

Author: tombruijn

packages/javascript/.changesets/add-callback-argument-to-senderror.md

@@ -0,0 +1,24 @@
+---
+bump: "patch"
+---
+
+Add callback argument to the `sendError` function to allow for more customization of errors sent with `sendError` and `wrap`. The `tags` and `namespace` parameters are now deprecated for both helpers.
+
+```js
+// Deprecated behavior
+appsignal.sendError(
+  new Error("sendError with tags and namespace argument"),
+  { tag1: "value 1", tag2: "value 2" },
+  "custom_namespace"
+);
+
+// New behavior
+appsignal.sendError(
+  new Error("sendError with callback argument"),
+  function(span) {
+    span.setAction("SendErrorTestAction");
+    span.setNamespace("custom_namespace");
+    span.setTags({ tag1: "value 1", tag2: "value 2" });
+  }
+);
+```

packages/javascript/src/__tests__/index.test.ts

@@ -74,6 +74,22 @@ describe("Appsignal", () => {
       expect(promise).resolves
     })
 
+    it("modifies the span before pushing the error", async () => {
+      const message = "test error"
+      let resultSpan: Span | undefined
+      const promise = appsignal.sendError(new Error(message), function (span) {
+        span.setAction("CustomAction")
+        resultSpan = span
+      })
+
+      await expect(promise).resolves
+      if (resultSpan) {
+        expect(resultSpan.serialize()?.action).toBe("CustomAction")
+      } else {
+        throw new Error("No resultSpan found!")
+      }
+    })
+
     it("doesn't send an invalid error", () => {
       const spy = jest.spyOn(console, "error").mockImplementation()
 
@@ -156,6 +172,28 @@ describe("Appsignal", () => {
 
       expect(promise).rejects
     })
+
+    it("modifies the span before pushing the error", async () => {
+      let resultSpan: Span | undefined
+      const promise = appsignal
+        .wrap(
+          () => {
+            throw new Error("test error")
+          },
+          function (span) {
+            span.setAction("CustomAction")
+            resultSpan = span
+          }
+        )
+        .catch(e => expect(e.message).toEqual("test error"))
+
+      await expect(promise).rejects
+      if (resultSpan) {
+        expect(resultSpan.serialize()?.action).toBe("CustomAction")
+      } else {
+        throw new Error("No resultSpan found!")
+      }
+    })
   })
 
   describe("createSpan", () => {

packages/javascript/src/index.ts

@@ -4,7 +4,7 @@
  */
 
 import { compose, toHashMap } from "@appsignal/core"
-import type { Breadcrumb, JSClient, Hook } from "@appsignal/types"
+import type { Breadcrumb, JSClient, Hook, HashMap } from "@appsignal/types"
 
 import { VERSION } from "./version"
 import { PushApi } from "./api"
@@ -70,6 +70,16 @@ export default class Appsignal implements JSClient {
     this._options = options
   }
 
+  /**
+   * Records and sends a browser `Error` to AppSignal.
+   *
+   * @param   {Error}             error          A JavaScript Error object
+   * @param   {Function | void}   fn             Optional callback function to modify span before it's sent.
+   *
+   * @return  {Promise<Span> | void}             An API response, or `void` if `Promise` is unsupported.
+   */
+  public send<T>(error: Error, fn?: (span: Span) => T): Promise<Span> | void
+
   /**
    * Records and sends a browser `Error` to AppSignal.
    *
@@ -97,14 +107,14 @@ export default class Appsignal implements JSClient {
   /**
    *
    * @param   {Error | Span}      data           A JavaScript Error or Appsignal Span object
-   * @param   {object}            tags           An key, value object of tags
-   * @param   {string}            namespace      An optional namespace name
+   * @param   {object | Function} tagsOrFn       An key-value object of tags or a callback function to customize the span before it is sent.
+   * @param   {string}            namespace      DEPRECATED: An optional namespace name.
    *
    * @return  {Promise<Span> | void}             An API response, or `void` if `Promise` is unsupported.
    */
-  public send(
+  public send<T>(
     data: Error | Span,
-    tags = {},
+    tagsOrFn?: object | ((span: Span) => T),
     namespace?: string
   ): Promise<any> | void {
     if (!(data instanceof Error) && !(data instanceof Span)) {
@@ -148,8 +158,25 @@ export default class Appsignal implements JSClient {
       compose(...this._hooks.decorators)(span)
     }
 
-    if (tags) span.setTags(tags)
-    if (namespace) span.setNamespace(namespace)
+    if (tagsOrFn) {
+      if (typeof tagsOrFn === "function") {
+        const callback = tagsOrFn
+        callback(span)
+      } else {
+        console.warn(
+          "[APPSIGNAL]: DEPRECATED: Calling the `send`/`sendError` function with a tags object is deprecated. Use the callback argument instead."
+        )
+        const tags = (toHashMap(tagsOrFn) || {}) as HashMap<string>
+        span.setTags(tags)
+      }
+    }
+    if (namespace) {
+      console.warn(
+        "[APPSIGNAL]: DEPRECATED: Calling the `send`/`sendError` function with a namespace is deprecated. Use the callback argument instead."
+      )
+      span.setNamespace(namespace)
+    }
+
     if (this._breadcrumbs.length > 0) span.setBreadcrumbs(this._breadcrumbs)
 
     // A Span can be "overridden" with metadata after it has been created,
@@ -194,22 +221,30 @@ export default class Appsignal implements JSClient {
     }
   }
 
+  sendError<T>(error: Error): Promise<Span> | void
+  sendError<T>(error: Error, callback: (span: Span) => T): Promise<Span> | void
+  sendError<T>(
+    error: Error,
+    tags?: object,
+    namespace?: string
+  ): Promise<Span> | void
+
   /**
    * Records and sends a browser `Error` to AppSignal. An alias to `#send()`
    * to maintain compatibility.
    *
    * @param   {Error}             error          A JavaScript Error object
-   * @param   {object}            tags           An key, value object of tags
-   * @param   {string}            namespace      An optional namespace name
+   * @param   {object | Function} tagsOrFn       An key-value object of tags or callback function to customize the span before it is sent.
+   * @param   {string}            namespace      DEPRECATED: An optional namespace name
    *
    * @return  {Promise<Span> | void}             An API response, or `void` if `Promise` is unsupported.
    */
-  public sendError(
+  public sendError<T>(
     error: Error,
-    tags?: object,
+    tagsOrFn?: object | ((span: Span) => T),
     namespace?: string
   ): Promise<Span> | void {
-    return this.send(error, tags, namespace)
+    return this.send(error, tagsOrFn, namespace)
   }
 
   /**
@@ -250,22 +285,44 @@ export default class Appsignal implements JSClient {
     return span
   }
 
+  /**
+   * Wraps and catches errors within a given function. If the function throws an
+   * error, a rejected `Promise` will be returned and the error thrown will be
+   * logged to AppSignal.
+   */
+  public async wrap<T>(fn: () => T, callbackFn?: (span: Span) => T): Promise<T>
+
+  /**
+   * Wraps and catches errors within a given function. If the function throws an
+   * error, a rejected `Promise` will be returned and the error thrown will be
+   * logged to AppSignal.
+   */
+  public async wrap<T>(
+    fn: () => T,
+    tags?: object,
+    namespace?: string
+  ): Promise<T>
+
   /**
    * Wraps and catches errors within a given function. If the function throws an
    * error, a rejected `Promise` will be returned and the error thrown will be
    * logged to AppSignal.
    *
    * @param   {Function}          fn             A function to wrap
-   * @param   {object}            tags           An key, value object of tags
-   * @param   {string}            namespace      An optional namespace name
+   * @param   {object | Function} tagsOrFn       An key-value object of tags or a callback function to customize the span before it is sent.
+   * @param   {string}            namespace      DEPRECATED: An optional namespace name
    *
    * @return  {Promise<any>}      A Promise containing the return value of the function, or a `Span` if an error was thrown.
    */
-  public async wrap<T>(fn: () => T, tags = {}, namespace?: string): Promise<T> {
+  public async wrap<T>(
+    fn: () => T,
+    tagsOrFn?: object | ((span: Span) => T),
+    namespace?: string
+  ): Promise<T> {
     try {
       return await fn()
     } catch (e) {
-      await this.sendError(e, tags, namespace)
+      await this.sendError(e, tagsOrFn, namespace)
       return Promise.reject(e)
     }
   }

packages/types/.changesets/add-callback-argument-to-senderror.md

@@ -0,0 +1,24 @@
+---
+bump: "patch"
+---
+
+Add callback argument to the `sendError` function to allow for more customization of errors sent with `sendError` and `wrap`. The `tags` and `namespace` parameters are now deprecated for both helpers.
+
+```js
+// Deprecated behavior
+appsignal.sendError(
+  new Error("sendError with tags and namespace argument"),
+  { tag1: "value 1", tag2: "value 2" },
+  "custom_namespace"
+);
+
+// New behavior
+appsignal.sendError(
+  new Error("sendError with callback argument"),
+  function(span) {
+    span.setAction("SendErrorTestAction");
+    span.setNamespace("custom_namespace");
+    span.setTags({ tag1: "value 1", tag2: "value 2" });
+  }
+);
+```

packages/types/src/interfaces/client.ts

@@ -87,6 +87,11 @@ export interface NodeClient extends BaseClient {
 export interface JSClient extends BaseClient {
   ignored: RegExp[]
 
+  /**
+   * Records and sends a browser `Error` to AppSignal.
+   */
+  send<T>(error: Error, fn?: (span: JSSpan) => T): Promise<JSSpan> | void
+
   /**
    * Records and sends a browser `Error` to AppSignal.
    */
@@ -103,6 +108,9 @@ export interface JSClient extends BaseClient {
     namespace?: string
   ): Promise<any> | void
 
+  sendError<T>(error: Error): Promise<JSSpan> | void
+  sendError<T>(error: Error, fn?: (span: JSSpan) => T): Promise<JSSpan> | void
+
   /**
    * Records and sends a browser `Error` to AppSignal. An alias to `#send()`
    * to maintain compatibility.
@@ -127,6 +135,13 @@ export interface JSClient extends BaseClient {
    */
   createSpan(fn?: (span: JSSpan) => void): JSSpan
 
+  /**
+   * Wraps and catches errors within a given function. If the function throws an
+   * error, a rejected `Promise` will be returned and the error thrown will be
+   * logged to AppSignal.
+   */
+  wrap<T>(fn: () => T, callbackFn?: (span: JSSpan) => T): Promise<T>
+
   /**
    * Wraps and catches errors within a given function. If the function throws an
    * error, a rejected `Promise` will be returned and the error thrown will be