BidsFile subclasses throw unwrapped IssueErrors from constructors

[happy5214]

This was brought up in another issue. I have since discovered that all BidsFile constructors throw IssueError objects containing regular HED Issue objects. This poses two issues:

1. The errors are not caught until reaching bids-validator, where they are presented as internal errors rather than well-defined HED errors.
2. The issues are not wrapped for the HED context, meaning any warnings would not be marked as such. Also, line numbers and other metadata are missing.

While properly returning actual BidsHedIssue objects from a constructor is impossible due to flow control restrictions, performing the wrapping operation should not be impossible.

The original explanation and report are below.

Yes, and you are right that it escapes uncaught. It is a deliberate throw rather
than a crash, from a constructor.

I patched the catch in bids-validator's src/validators/hed.ts to dump the
error, then ran probe 3 (hed-validator 4.2.0, deno 2.9.4):

name:        IssueError
own keys:    [ "issue", "name" ]
message:     ERROR: [SIDECAR_INVALID] The string 'HED' or 'n/a' was illegally used
             as a top-level sidecar key. Sidecar key: "HED".
stack:
    at IssueError.generateAndThrow (hed-validator/dist/esm/index.js:44:11)
    at                             (hed-validator/dist/esm/index.js:191:24)
    at Array.map (<anonymous>)
    at BidsSidecar._filterHedStrings (hed-validator/dist/esm/index.js:187:10)
    at new BidsSidecar             (hed-validator/dist/esm/index.js:114:10)
    at new BidsTsvFile             (hed-validator/dist/esm/index.js:50:26)
    at buildHedTsvFile (bids-validator/src/validators/hed.ts:120:10)
    at hedValidate     (bids-validator/src/validators/hed.ts:82:14)

Deno resolves those through the bundle's source map, so the positions are
original-source ones with the filename collapsed to index.js. Resolved against
the map they are:

• src/bids/types/json.js:191, IssueError.generateAndThrow('illegalSidecarHedKey', …), inside _filterHedStrings (:187)
• src/bids/types/json.js:114, this._filterHedStrings() in the BidsSidecar constructor
• src/bids/types/tsv.js:50, this.mergedSidecar = new BidsSidecar(…) in the BidsTsvFile constructor

So it is thrown while constructing the file object, before file.validate() is
reached, and bids-validator's only handler is the catch-all in hedValidate.
That is why a well-defined SIDECAR_INVALID arrives as INTERNAL_ERROR: there
is no issue list in scope yet to return it on. @throws {IssueError} is
documented on the neighbouring _verifyKeyHasNoDeepHed, so throwing during
construction looks intended, which would make the fix surfacing construction
issues on the object rather than catching harder downstream.

Originally posted by @ttm in #836

[ttm]

Thanks for splitting this out. Inventory in case it saves you a pass. On main
(61d0922, 4.2.0), six codes can escape a constructor:

Site	Code	Reached from
json.js:173	invalidDefinitionManager	_setDefinitions, ctor :113
json.js:191	illegalSidecarHedKey	_filterHedStrings, ctor :114
json.js:219	illegalSidecarHedDeepKey	_verifyKeyHasNoDeepHed, from _filterHedStrings:201
json.js:388	illegalSidecarHedType	new BidsSidecarKey, from _filterHedStrings:198
json.js:278	illegalSidecarData	_generateSidecarColumnSpliceMap, from _categorizeHedStrings:158
tsv.js:47	internalError	the BidsTsvFile constructor itself

BidsTsvFile inherits all five sidecar ones through new BidsSidecar at
tsv.js:50.

The other three generateAndThrow sites under src/bids (json.js:448,
json.js:453, sidecarValidator.js:74) are validation-time and already come out
wrapped, because BidsFile.validate catches and returns
BidsHedIssue.fromHedIssues(error, this.file) at file.js:66. So both the
wrapper and the file handle it needs already exist one seam down, and
construction is just the path with no try around it, which I think supports
your read that wrapping is doable where returning is not.

Happy to run any branch against the six probe datasets from #836.