fix(shared): throw UnknownAttachmentDataError for unhandled AttachmentDescriptor payloads

[abhayrajjais01]

Description

AttachmentDescriptor.extractJSON threw a generic Error("Unhandled attachment") for payloads that were neither valid base64 JSON nor a recognizable json attachment shape. The domain layer already defines MercuryError.UnknownAttachmentDataError, but this path did not use it, so callers could not reliably branch on Mercury’s typed errors.

This PR throws UnknownAttachmentDataError from that fallback path with a short message describing the expected shapes, and adds a Vitest regression test (header-only attachment data) that expects MercuryError.UnknownAttachmentDataError.

Files: packages/shared/domain/src/models/MessageAttachment.ts, packages/shared/domain/tests/setup.ts, packages/shared/domain/tests/MessageAttachment.test.ts.

Alternatives Considered (optional)

• Keep Error: Preserves old instanceof Error behavior but loses the domain error type and is inconsistent with existing Mercury error classes.
• New error class: Unnecessary; UnknownAttachmentDataError already matches the semantic.

Checklist

• My PR follows the contribution guidelines of this project
• My PR is free of third-party dependencies that don't comply with the Allowlist
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• I have checked the PR title to follow the conventional commit specification

[elribonazo]

Nothing that you added should have any effect on the setup.ts files, why did you add that?

[elribonazo]

Is that needed really? Nothing that u added has any effect on this, am I wrong?

[abhayrajjais01]

I added tests/setup.ts only because vitest.config.js already references ./tests/setup.ts but that file was missing on main -> I can keep an empty/minimal setup file for that path, without the polyfill,
Buffer was already used in MessageAttachment, and Vitest’s jsdom env still runs under Node where Buffer is normally global, so I wil remove the buffer import and global assignment

[abhayrajjais01]

@elribonazo ,
what should i do??

1st - remove the Buffer and if-else block and write a tiny placeholder file so vitest.config.js still has a real ./tests/setup.ts

or
delete setup.ts and remove the setupFiles: [tests/setup.ts] line from vitest.config.js so Vitest doesn’t point at a missing file??

[yshyn-iohk]

Thanks for your contribution, @abhayrajjais01!

The change itself is correct and clean. MessageAttachment.ts line 98–99 had:

// TODO better error
throw new Error("Unhandled attachment");

The domain layer already defines MercuryError.UnknownAttachmentDataError (in packages/shared/domain/src/models/errors/Mercury.ts:37) for exactly this semantic. The PR simply uses it — this is the right call. Since UnknownAttachmentDataError extends Error, any existing catch (e) blocks that check instanceof Error remain unaffected, so it's backward compatible.

The test is clean — it constructs a header-only AttachmentDescriptor (which is neither base64 nor JSON), calls extractJSON, and expects the typed error. Good regression coverage.

The setup.ts question

Javi's pushback is valid — the content of setup.ts (Buffer polyfill) is unnecessary. But the file does need to exist, and here's why:

packages/shared/domain/vitest.config.js already has:

setupFiles: ['./tests/setup.ts'],
passWithNoTests: true,

On main today, there are no test files in packages/shared/domain/tests/ — the directory doesn't even exist. Since passWithNoTests: true means vitest skips execution entirely when there are no matching test files, the missing setup.ts has never been a problem. But this PR adds the first test (MessageAttachment.test.ts), so vitest will now actually run — and it will fail when trying to load the missing setup.ts.

The right path forward: keep setup.ts as an empty file (or a single comment like // vitest setup), remove the Buffer polyfill. This satisfies the vitest config reference without adding unnecessary code.

// packages/shared/domain/tests/setup.ts
// intentionally empty – required by vitest.config.js setupFiles

[coveralls]

coverage: 75.224% (+0.03%) from 75.19% — abhayrajjais01:fix/shared-unknown-attachment-error into hyperledger-identus:main

[abhayrajjais01]

@yshyn-iohk,
Applied your suggestion
Thank you

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[elribonazo]

@abhayrajjais01 please check your commit signatures

[elribonazo]

Please remove

[elribonazo]

Closing due to inactivity

[abhayrajjais01]

hi @elribonazo,
created a new pr addressing your suggestions, please take a look #642