Open BebeSparkelSparkel opened 6 years ago
I have a feeling it's because of https://github.com/sindresorhus/aggregate-error/blob/7ae39e2dacb1f6a800fca7fb271e43da79e04d6f/index.js#L24-L28 but I'm not sure what Mocha is doing to trigger it.
// @boneskull
good question!
whoever wants to look into this, please create an issue and PR on Mocha's issue tracker. The problem is likely somewhere in this function.
I think one problem is that aggregate-error puts all the stacks of the errors into message
, but imo message
should only contain the aggregated message
s. The aggregated stacks with messages could be returned when .toString()
is invoked, just like with regular Errors.
Tested Chrome does not invoke toString()
for errors, but it uses stack
, and it's possible to assign this.stack
to a custom concatenation of everything
Yeah, I want to align this better with https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError/AggregateError
.message
just the message of the AggregateError, and fix up the .stack
..errors
property instead of the instance being iterable.message
argument: https://github.com/sindresorhus/aggregate-error/issues/12I'll be watching this one--Mocha should understand these errors.
@sindresorhus this setup is arguably better than the spec :(
@timoxley How so?
tbh stuffing the stack(s) into message
seems like misuse of the prop
@sindresorhus Spec AggregateError
displays ok in node because it also prints the error's own properties via util.inspect
, but errors
are entirely hidden in the browser console using console.log
or console.error
:
But does give better logging with console.dir
So I guess the problem is really more of a shortcoming of the browser console
implementation rather than AggregateError
itself. Nevermind
@sindresorhus @timoxley I do think this is better in some ways than the spec. This library currently creates a sensible error message if a custom one isn't provided. With the official AggregateError:
vs current situation (and what the docs will have led users to do already in existing codebases):
So making it spec compliant would involve changing the message-less behaviour to make the resultant message prop less helpful. That might hurt a lot of users who auto-update when they get paged in the middle of the night with Alert! Error in production: ""
.
Given that, IMO making it spec compliant should be either a major release and make message
required... or a non-goal - separate out functionality from the spec and rename to multi-error
or something. As a user I'd prefer the latter. This library can be more useful than a polyfill (/ponyfill) of something coming to ES anyway.
One idea would be to turn this module into a factory function createAggregateError()
for the standard global AggregateError
that does the smarter error messages. It could also make sure error instances are passed. It could do more than the current one even, e.g. if only one error is passed return it as-is (don't know if that's a good idea). And then maybe provide some additional functions for dealing with AggregateError
s, e.g. a type guard isAggregateError()
.
I found this because I was looking for something roughly equivalent to verror, which is good but hasn't been published in four years. It'd be nice to have a semi-official way of aggregating errors. But AggregateError
as defined in the ES spec is a bit limited. Maybe this library could go further in the verror direction rather than towards the ES spec, and provide helpers for wrapping errors etc.?
When used with mocha the errors are displayed twice. Is there a way to have it only display once?
The report is