The data property of a Boom error gets mangled when the error comes from a validator

[hs-kgoriunov]

Runtime

node.js

Runtime version

v24.12.0

Module version

v21.4.9

Last module version without issue

v20.3.0

Used with

hapi application

Any other relevant information

I believe #4350 introduced an edge case issue.

What are you trying to achieve or the steps to reproduce?

Here is a distilled test to showcase the issue.

import Boom from "@hapi/boom"; // 10.0.1
import Hapi from "@hapi/hapi"; // 21.4.9
import Joi from "joi"; // 18.2.3

import assert from "node:assert";
import { afterEach, beforeEach, describe, it } from "node:test";

describe("error mangling showcase", () => {
  let server;
  const myError = Boom.badRequest("My message", { my: "data" });

  beforeEach(async () => {
    // Initialize Hapi
    server = Hapi.server();
    await server.initialize();

    // Set up a basic global error response formatter to expose the error's data property
    server.ext("onPreResponse", (request, h) => {
      const { response } = request;

      if (response.isBoom) {
        const error = response;
        error.output.payload = {
          error: {
            message: error.message,
            data: error.data,
          },
        };
      }

      return h.continue;
    });
  });

  afterEach(async () => {
    await server.stop();
  });

  it("throws myError from handler", async () => {
    server.route({
      method: "GET",
      path: "/",
      handler: function () {
        throw myError;
      },
    });

    const res = await server.inject({
      method: "get",
      url: "/",
    });

    // This assertion passes regarless of Hapi version
    assert.deepStrictEqual(res.result, {
      error: {
        message: "My message",
        data: { my: "data" },
      },
    });
  });

  it("throws myError from validation schema", async () => {
    server.route({
      method: "GET",
      path: "/",
      handler: function () {
        return "Hello world!";
      },
      options: {
        validate: {
          query: Joi.object({
            dummy: Joi.required().error(myError),
          }),
        },
      },
    });

    const res = await server.inject({
      method: "get",
      url: "/",
    });

    // This assertion fails on Hapi v21 and onward
    assert.deepStrictEqual(res.result, {
      error: {
        message: "My message",
        // the data is { defaultError: myError } starting from Hapi v21
        data: { my: "data" },
      },
    });
  });
});

What was the result you got?

$ node --test example.mjs 
▶ issue showcase
  ✔ throws myError from handler (23.230669ms)
  ✖ throws myError from validation schema (11.012993ms)
✖ issue showcase (35.352659ms)
ℹ tests 2
ℹ suites 1
ℹ pass 1
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 399.773655

✖ failing tests:

test at example.mjs:62:3
✖ throws myError from validation schema (11.012993ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
  + actual - expected
  
  + <ref *2> {
  - {
      error: {
  +     data: <ref *1> {
  +       defaultError: Error: My message
  +           at SuiteContext.<anonymous> (file:///[redacted]/example.mjs:10:24)
  +           at Suite.runInAsyncScope (node:async_hooks:214:14)
  +           at Suite.createBuild (node:internal/test_runner/test:1478:13)
  +           at new Suite (node:internal/test_runner/test:1469:28)
  +           at Test.createSubtest (node:internal/test_runner/test:865:18)
  +           at run (node:internal/test_runner/harness:364:28)
  +           at test (node:internal/test_runner/harness:378:12)
  +           at file:///[redacted]/example.mjs:8:1
  +           at ModuleJob.run (node:internal/modules/esm/module_job:413:25)
  +           at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:660:26) {
  +         data: [Circular *1],
  +         isBoom: true,
  +         isServer: false,
  +         output: {
  +           headers: {},
  +           payload: [Circular *2],
  +           statusCode: 400
  +         }
  +       }
  -     data: {
  -       my: 'data'
        },
        message: 'My message'
      }
    }
  
      at TestContext.<anonymous> (file:///[redacted]/example.mjs:83:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
      at async Test.run (node:internal/test_runner/test:1113:7)
      at async Suite.processPendingSubtests (node:internal/test_runner/test:788:7) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: { error: { message: 'My message', data: [Object] } },
    expected: { error: { message: 'My message', data: [Object] } },
    operator: 'deepStrictEqual',
    diff: 'simple'
  }

What result did you expect?

$ node --test example.mjs
▶ issue showcase
  ✔ throws myError from handler (35.244367ms)
  ✔ throws myError from validation schema (12.917042ms)
✔ issue showcase (50.673704ms)
ℹ tests 2
ℹ suites 1
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 718.821985

[kanongil]

It is definitely unfortunate that validation errors get their data property rewritten.

I'm not sure how to fix this though. It seems that adding a defaultError property to the data object was a mistake in the first place, as neither overwriting, merging, or preserving an existing data object seems appropriate. If it was implemented today, I would put it in the error.cause property which is there to expose underlying errors.