Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for (or document) Error#cause #50

Closed
fregante opened this issue Sep 26, 2021 · 4 comments · Fixed by #65
Closed

Add support for (or document) Error#cause #50

fregante opened this issue Sep 26, 2021 · 4 comments · Fixed by #65

Comments

@fregante
Copy link
Contributor

Context:

I think this already kind of supported, but it should probably be documented and tested, especially if #48 is implemented

@mayorandrew
Copy link

There is library make-error-cause which allows to create Errors with causes. It is fairly popular (600k downloads per week). The cause property is defined as enumerable: false there, so it is not automatically picked up by serialize-error.

I currently use custom toJSON method on the base error class to include cause property in the serialized object, like so:

import { BaseError as BaseMakeError } from 'make-error-cause';
import { serializeError } from 'serialize-error';
class BaseError extends BaseMakeError {
  public toJSON() {
    return {
      ...serializeError(this),
      cause: serializeError(this.cause)
    };
  }
}

Kudos for isCalled btw, it really helped here.

Still it would be nice if cause was included in the commonProperties list. Or even better, if commonProperties list was customizable via options on serializeError.

If that is something you're interested in, I can make a PR.

@mayorandrew
Copy link

I went ahead and tried to implement this, but the line
https://github.com/sindresorhus/serialize-error/blob/main/index.js#L106

if (typeof from[property] === 'string') {

is in the way of serializing the cause via commonProperties.
As I understand that is because of deserialization back to Error. I am not sure how to handle deserialization for cause though. I think this problem is related to #46, so some design decision should be made here to keep everything coherent.

Maybe destroyCircular could accept deserialize: true flag to provide different behavior for serialization and deserialization.

@fregante
Copy link
Contributor Author

To add more context, here's the current support:

  • serialization: fully works with the native cause property
  • deserialization: error.cause is preserved but not deserialized

This is the result of:

// Extra wrappers to test the stack visibility as well
function throwing () {
	throw new TypeError('original error')
}

function catcher() {
	try {
		throwing()
	} catch (typeError) {
		const extendedError = new Error('Wrapped error with more info')
		extendedError.cause = typeError;
		throw extendedError;
	}
}

async function init() {
	catcher()
}

init().catch(error => {
	console.log({
		error, 
		serialized: serializeError(error),
		seriaDeseria: deserializeError(serializeError(error))
	})
})

Screen Shot 1

@fregante
Copy link
Contributor Author

fregante commented Mar 25, 2022

Correction:

  • the native Error#cause does not appear in serialized errors

My example worked because I set the cause property directly in an env that does not support Error#cause natively. The difference is that the native cause is not enumerable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants