fix(catchers): validate beforeSend return value to avoid sending invalid payload #150

Dobrunia · 2026-01-28T15:09:24Z

Problem

When beforeSend returns a non-object (e.g. true, undefined, or omits return), that value was used as payload and sent to the collector. The backend then stored it as-is (e.g. payload: true in MongoDB), which led to:

GraphQL: Cannot return null for non-nullable field EventPayload.title
Frontend: TypeError: can't access property "event", l is null on project overview

Common mistakes:

beforeSend: (e) => true (meant “allow”, but overwrites payload)
beforeSend: (e) => { console.log(e); } (no return → undefined as payload)

Solution

Validate the return value of beforeSend and only use it when it’s a valid event object:

…lid payload

…n for beforeSend and beforeBreadcrumb return values

Copilot

Pull request overview

This PR hardens the JavaScript SDK hook handling so beforeSend (and similarly beforeBreadcrumb) can’t accidentally cause invalid payloads/breadcrumbs to be sent/stored when the hook returns an unexpected value.

Changes:

Add runtime validators (isValidEventPayload, isValidBreadcrumb) and apply them to hook results.
Update hook typings/docs to allow void return (keep original) and use false to explicitly drop events/breadcrumbs.
Add/adjust examples and docs to demonstrate the new hook behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
packages/javascript/src/utils/validation.ts	Adds runtime validation helpers for event payloads and breadcrumbs.
packages/javascript/src/types/hawk-initial-settings.ts	Updates `beforeSend` typing/docs to include `void` return behavior.
packages/javascript/src/catcher.ts	Validates `beforeSend` return value before overwriting the outgoing payload; adds warnings for invalid returns.
packages/javascript/src/addons/breadcrumbs.ts	Validates `beforeBreadcrumb` return value; updates hook behavior/docs and warnings.
packages/javascript/package.json	Bumps package version to 3.2.16.
packages/javascript/example/index.html	Updates example to use `false` (instead of `null`) for discarding breadcrumbs.
packages/javascript/example/hooks-tests.html	Adds a manual test page for hook scenarios.
packages/javascript/example/before-send-tests.js	Adds manual test script to exercise hook return cases.
packages/javascript/README.md	Updates public docs for hook return semantics and examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T14:11:37Z

packages/javascript/src/utils/validation.ts

+    return false;
+  }
+
+  if (payload.backtrace !== undefined && !Array.isArray(payload.backtrace)) {


isValidEventPayload treats backtrace: null as invalid because it only allows an array when the field is present. In this SDK, HawkJavaScriptEvent['backtrace'] is BacktraceFrame[] | null, so a payload returned from beforeSend that keeps backtrace as null will be rejected and silently ignored. Update the check to allow null (and/or align the runtime validation with HawkJavaScriptEvent’s actual shape).

Suggested change

if (payload.backtrace !== undefined && !Array.isArray(payload.backtrace)) {

if (payload.backtrace !== undefined && payload.backtrace !== null && !Array.isArray(payload.backtrace)) {

Copilot · 2026-02-11T14:11:38Z

packages/javascript/src/utils/validation.ts

+ * @param breadcrumb - value to validate
+ */
+export function isValidBreadcrumb(breadcrumb: unknown): breadcrumb is Breadcrumb {
+  if (typeof breadcrumb !== 'object' || breadcrumb === null || Array.isArray(breadcrumb)) {


The JSDoc says the breadcrumb must be a plain object, but isValidBreadcrumb currently accepts any non-null, non-array object (e.g. new Date()), which doesn’t match the documented behavior. Either tighten the check (e.g. reuse isPlainObject) or adjust the comment to reflect what’s actually validated.

Suggested change

if (typeof breadcrumb !== 'object' || breadcrumb === null || Array.isArray(breadcrumb)) {

if (!isPlainObject(breadcrumb)) {

Copilot · 2026-02-11T14:11:38Z

packages/javascript/src/catcher.ts

+   * - Return nothing (`void` / `undefined` / `null`) — the original event is sent as-is (a warning is logged).
   */
-  private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false);
+  private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false | void);


The comment mentions that beforeSend may return null, and the runtime code handles null, but the TypeScript type doesn’t include null (HawkJavaScriptEvent | false | void). Either include null in the return type or remove null from the docs/handling so the public API is consistent.

Suggested change

private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false | void);

private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false | void | null);

Copilot · 2026-02-11T14:11:38Z

packages/javascript/src/catcher.ts

+       * If user returned nothing (void/undefined/null) — warn and keep original payload
+       */
+      if (result === undefined || result === null) {
+        log(`[Hawk] Invalid beforeSend value: (${String(result)}). It should return event or false. Event is sent without changes.`, 'warn');


The warning says the event is sent “without changes”, but beforeSend receives the actual payload object—any in-place mutations done before returning undefined/null will still be sent. To make this accurate, either clone/freeze the payload before calling beforeSend (and only use the clone when the hook returns a valid event) or reword the warning to not promise that no changes were applied.

Suggested change

* If user returned nothing (void/undefined/null) — warn and keep original payload

*/

if (result === undefined || result === null) {

log(`[Hawk] Invalid beforeSend value: (${String(result)}). It should return event or false. Event is sent without changes.`, 'warn');

* If user returned nothing (void/undefined/null) — warn and send current payload as is

*/

if (result === undefined || result === null) {

log(`[Hawk] Invalid beforeSend value: (${String(result)}). It should return event or false. Event will be sent as is.`, 'warn');

Copilot · 2026-02-11T14:11:39Z

packages/javascript/src/addons/breadcrumbs.ts

+      /**
+       * false means discard
+       */
+      if (result === false) {
        return;
      }


beforeBreadcrumb previously used null to discard breadcrumbs, but this change switches the sentinel to false and now treats null as “no return” (stores the breadcrumb). That’s a breaking behavior change for existing consumers; consider continuing to treat null as discard (possibly with a deprecation warning) or bump the package with an appropriate breaking-change version.

Copilot · 2026-02-11T14:11:39Z

packages/javascript/src/addons/breadcrumbs.ts

+   * - If the hook returns an invalid value, a warning is logged and the original breadcrumb is stored.
   */
-  beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | null;
+  beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | void;


The JSDoc says beforeBreadcrumb can return null, and the runtime path handles null, but the callback type is Breadcrumb | false | void (no null). Align the public type signature with the documented/runtime behavior (either add null or remove null from docs and handling).

Suggested change

beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | void;

beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | null | void;

Copilot · 2026-02-11T14:11:39Z

packages/javascript/src/addons/breadcrumbs.ts

+       * void/undefined/null — warn and keep original breadcrumb
+       */
+      if (result === undefined || result === null) {
+        log('[Hawk] beforeBreadcrumb returned nothing, storing original breadcrumb.', 'warn');
+      } else if (isValidBreadcrumb(result)) {
+        Object.assign(bc, result);
+      } else {
+        log(
+          '[Hawk] beforeBreadcrumb produced invalid breadcrumb (must be an object with numeric timestamp), storing original. '


The log message says it’s “storing original breadcrumb”, but beforeBreadcrumb is called with the live bc object, so in-place mutations will still be stored even when the hook returns undefined/null or an invalid value. Consider cloning bc before invoking the hook or reword the warning to avoid implying the breadcrumb is unchanged.

Suggested change

* void/undefined/null — warn and keep original breadcrumb

*/

if (result === undefined || result === null) {

log('[Hawk] beforeBreadcrumb returned nothing, storing original breadcrumb.', 'warn');

} else if (isValidBreadcrumb(result)) {

Object.assign(bc, result);

} else {

log(

'[Hawk] beforeBreadcrumb produced invalid breadcrumb (must be an object with numeric timestamp), storing original. '

* void/undefined/null — warn and keep breadcrumb as last modified by hook

*/

if (result === undefined || result === null) {

log('[Hawk] beforeBreadcrumb returned nothing; keeping breadcrumb as last modified by hook.', 'warn');

} else if (isValidBreadcrumb(result)) {

Object.assign(bc, result);

} else {

log(

'[Hawk] beforeBreadcrumb produced invalid breadcrumb (must be an object with numeric timestamp); keeping breadcrumb as last modified by hook. '

Copilot · 2026-02-11T14:11:40Z

packages/javascript/src/types/hawk-initial-settings.ts

+   * - Return nothing (`void` / `undefined` / `null`) — the original event is sent as-is (a warning is logged).
   */
-  beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false;
+  beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false | void;


beforeSend docs mention null as a possible “no return” value, but the declared type is HawkJavaScriptEvent | false | void (no null). If null is intended to be treated like undefined, add null to the signature; otherwise remove it from the docs to keep the API contract consistent.

Suggested change

beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false | void;

beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false | void | null;

packages/javascript/example/before-send-tests.js

fix(catchers): validate beforeSend return value to avoid sending inva…

9bd6a90

…lid payload

Dobrunia mentioned this pull request Jan 28, 2026

fix(workers): reject events with non-object payload to avoid corrupted DB records codex-team/hawk.workers#515

Open

e11sy approved these changes Feb 2, 2026

View reviewed changes

Dobrunia added 3 commits February 11, 2026 16:26

chore: update version to 3.2.16 in package.json and enhance validatio…

f20a748

…n for beforeSend and beforeBreadcrumb return values

chore: tests

da99f8a

Merge branch 'master' into fix/prevent-invalid-payload-from-beforeSend

17f43e2

neSpecc requested a review from Copilot February 11, 2026 14:07

Copilot started reviewing on behalf of neSpecc February 11, 2026 14:07 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

neSpecc reviewed Feb 11, 2026

View reviewed changes

packages/javascript/example/before-send-tests.js Outdated Show resolved Hide resolved

Dobrunia added 4 commits February 11, 2026 17:46

chore: add tests

14cbeab

fix

e79e9c3

fix

c1a81f2

fix: lint

5505090

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(catchers): validate beforeSend return value to avoid sending invalid payload #150

fix(catchers): validate beforeSend return value to avoid sending invalid payload #150

Dobrunia commented Jan 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (payload.backtrace !== undefined && !Array.isArray(payload.backtrace)) {
	if (payload.backtrace !== undefined && payload.backtrace !== null && !Array.isArray(payload.backtrace)) {

	if (typeof breadcrumb !== 'object' \|\| breadcrumb === null \|\| Array.isArray(breadcrumb)) {
	if (!isPlainObject(breadcrumb)) {

	private readonly beforeSend: undefined \| ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent \| false \| void);
	private readonly beforeSend: undefined \| ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent \| false \| void \| null);

	beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb \| false \| void;
	beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb \| false \| null \| void;

	beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent \| false \| void;
	beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent \| false \| void \| null;

fix(catchers): validate beforeSend return value to avoid sending invalid payload #150

Are you sure you want to change the base?

fix(catchers): validate beforeSend return value to avoid sending invalid payload #150

Conversation

Dobrunia commented Jan 28, 2026

Problem

Solution

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants