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

Circular reference stacktrace very painful for development #40

Open
saibotsivad opened this issue Aug 3, 2022 · 0 comments
Open

Circular reference stacktrace very painful for development #40

saibotsivad opened this issue Aug 3, 2022 · 0 comments

Comments

@saibotsivad
Copy link

When running across an accidental circular reference (aka uncovering a bug) buried deep in a stack, calling klona throws a Maximum call stack size exceeded but with the stacktrace only containing the last n lines from within klona, meaning that there's no way to see what the call stack is that called klona. This is a frustrating developer experience.

Note: this is different than #37 which wants a way to do something. I only want a useful stacktrace so I can track down and handle the error.

An example

Imagine that you have a bug deep in your API stack that creates a circular reference:

// circular.js
const dog = { type: 'dog' }
const person = { type: 'person' }
dog.owner = person
person.pet = dog

Making a naive copy of person e.g. JSON.parse(JSON.stringify(person)) will of course throw a circular reference exception, notably with the stacktrace intact so you can see where the clone was created and the error was thrown:

console.log(JSON.parse(JSON.stringify(person)))
                                     ^

TypeError: Converting circular structure to JSON
[...snip...]
    at file:///circular.js:5:38
[...snip...]

However, when cloning person with any of the klona importables, e.g.:

import { klona } from 'klona/json'
// ...
console.log(klona(person))

The stacktrace throws a RangeError with the stacktrace entirely filled with these:

RangeError: Maximum call stack size exceeded
    at Object.toString (<anonymous>)
    at klona (/my-app/node_modules/klona/json/index.mjs:10:32)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)
    at klona (/my-app/node_modules/klona/json/index.mjs:6:66)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)
    at klona (/my-app/node_modules/klona/json/index.mjs:21:56)

Workaround

My current workaround in any of my apps is to basically wrap klona with a try/catch and re-throw a new error:

// clone.js
import { klona } from 'klona/json'
export const clone = obj => {
  try {
    return klona(obj)
  } catch(error) {
    throw new Error('Got an error in klona: ' + error.message)
  }
}

// circular.js
import { clone } from './clone.js'
const dog = { type: 'dog' }
const person = { type: 'person' }
dog.owner = person
person.pet = dog
console.log(clone(person))

which then throws this, which has the useful bits of the stacktrace in the error:

file:///clone.js:6
		throw new Error('Got an error in klona: ' + error.message)
		      ^

Error: Got an error in klona: Maximum call stack size exceeded
    at clone (file:///clone.js:6:9)
    at file:///circular.js:8:13

Issue

Of course, this is a really annoying developer experience, since I have to choose between either try/catch-ing for every klona call (with the wrapper) or completely losing the ability to find where the klona call is happening that has a circular reference.

For the most part I've been able to track down bugs without the wrapper, knowing that I'm working in a particular flow so the problem should be around ~here, but as the application has grown in complexity it's more frustrating to run into this issue, which is why I made the wrapper and came here.

Solution?

Probably the best solution would be to split the exported function and the recursed function, so the try catch happens only on the one exported function instead of every recursed call:

function _klona(val) {
	// the existing code, except recursion calls _klona
}
export function klona(val) {
	try {
		return _klona(val)
	} catch (error) {
		if (error.name === 'RangeError') throw new Error('Circular reference detected')
		else throw error
	}
}

I'm okay using that little wrapper in my apps, but it would be a nicer developer experience to have it built in.

Question

If you like that solution, I can make the switch and add tests etc. if you are open to that.

If not just say so here and feel free to close this issue.

(In part I'm filing an issue so that when this comes up again I'll see this and not keep bugging you about it. 😅)

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

No branches or pull requests

1 participant