rlaffers / xstate-ninja

Devtool for XState state machines
ISC License
69 stars 2 forks source link

Circular structure in context crash the machine #10

Closed Zehir closed 1 year ago

Zehir commented 1 year ago

Hello,

I have a machine where I store a rest client inside the context but it's seems to have circular structure and this crash the xstate-ninja and then the machine;

Uncaught (in promise) TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Client'
    |     property 'admins' -> object with constructor 'AdminService'
    --- property 'client' closes the circle
    at JSON.stringify (<anonymous>)
    at new H (xstate-ninja.js:181:36)
    at xstate-ninja.js:398:17
    at Interpreter2.update (interpreter.js:452:9)
    at interpreter.js:106:15
    at Scheduler2.process (scheduler.js:65:7)
    at Scheduler2.schedule (scheduler.js:44:10)
    at Interpreter2.send (interpreter.js:100:23)
    at _a2.id.id (interpreter.js:1135:15)
H @ xstate-ninja.js:181
(anonyme) @ xstate-ninja.js:398
Interpreter2.update @ interpreter.js:452
(anonyme) @ interpreter.js:106
Scheduler2.process @ scheduler.js:65
Scheduler2.schedule @ scheduler.js:44
Interpreter2.send @ interpreter.js:100
_a2.id.id @ interpreter.js:1135

Machine used ;

import { assign, createMachine } from 'xstate'
import PocketBase from 'pocketbase'
import { produce } from 'immer'

export interface StoreContext {
  pocketBaseUrl: string
  pocketBase?: PocketBase
}

export const storeMachine = createMachine({
  id: 'store',

  predictableActionArguments: true,
  tsTypes: {} as import('./store.typegen').Typegen0,

  schema: {
    context: {} as StoreContext,
    events: {} as {
      type: 'Add gateway'
    },
    services: {} as {
      connectToPocketBase: {
        data: PocketBase
      }
    },
  },

  context: {
    pocketBaseUrl: import.meta.env.VITE_POCKETBASE_URL,
  },

  states: {
    init: {
      after: {
        500: 'connecting',
      },
    },

    connecting: {
      invoke: {
        src: 'connectToPocketBase',
        onDone: {
          target: 'online',
          actions: 'usePocketBase',
        },
        onError: 'offline',
      },
    },

    online: {},
    offline: {},
  },

  initial: 'init',
},
{
  actions: {
    usePocketBase: assign({
      pocketBase: (context, event) => produce(context.pocketBase, (draft) => {
        draft = event.data
      }),
    }),
  },
  services: {
    connectToPocketBase: async (context) => {
      const client = new PocketBase(context.pocketBaseUrl)
      const health = await client.health.check()
      if (health.code === 200)
        return client
      else
        throw new Error('Could not connect to PocketBase')
    },
  },
})

With XStateNinja registred I am stuck in the connecting state and without I get to the online state

rlaffers commented 1 year ago

Yes, I have just encountered a related problem. This has to do with the serialization of events done by xstate-ninja prior to sending them to the browser extension. There is a sanitizeObject function which tries to strip the event objects off the unsafe properties, but apparently it fails. https://github.com/rlaffers/xstate-ninja/blob/master/packages/xstate-ninja-core/src/utils.ts#L145

We need to think about a better strategy.

rlaffers commented 1 year ago

A hotfix would be always stripping away some properties, e.g. those starting with _. E.g.:

export function sanitizeObject<T extends Record<string, any>>(obj: T): T {
  const result: Record<string, any> = {}
  for (const key in obj) {
    if (key[0] === '_') {
      // skip properties starting with _
      result[key] = '<stripped>'
      continue
    }

I'm not a big fan of doing this, because it requires a change in the machine configuration (prefixing unsafe event props with "_"). What do you think?

rlaffers commented 1 year ago

Of course, it would be also possible to traverse everything and detect circular references, but this may be costly.

Zehir commented 1 year ago

A hotfix would be always stripping away some properties, e.g. those starting with _. E.g.:

export function sanitizeObject<T extends Record<string, any>>(obj: T): T {
  const result: Record<string, any> = {}
  for (const key in obj) {
    if (key[0] === '_') {
      // skip properties starting with _
      result[key] = '<stripped>'
      continue
    }

I'm not a big fan of doing this, because it requires a change in the machine configuration (prefixing unsafe event props with "_"). What do you think?

I can't control the library so it's hard to do, maybe limit the depth of the context or query one level when expending in the extension

rlaffers commented 1 year ago

Indeed. I will implement a proper function for filtering out circular refs.

Zehir commented 1 year ago

Indeed. I will implement a proper function for filtering out circular refs.

Is there any work around that I can use today ?

rlaffers commented 1 year ago

@Zehir Try upgrading to xstate-ninja@1.3.5

Zehir commented 1 year ago

Hello @rlaffers I still have the error;

xstate-ninja.js:114 Uncaught (in promise) TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Client'
    |     property 'recordServices' -> object with constructor 'Object'
    |     property 'user' -> object with constructor 'RecordService'
    --- property 'client' closes the circle
    at JSON.stringify (<anonymous>)
    at C (xstate-ninja.js:114:17)
    at new ee (xstate-ninja.js:266:19)
    at xstate-ninja.js:482:17
    at Interpreter2.update (interpreter.js:452:9)
    at interpreter.js:106:15
    at Scheduler2.process (scheduler.js:65:7)
    at Scheduler2.schedule (scheduler.js:44:10)
    at Interpreter2.send (interpreter.js:100:23)
    at Interpreter2.inspectSend [as send] (browser.js:126:24)
rlaffers commented 1 year ago

@Zehir Can you perhaps set up a Codesandbox for this? Or else provide what is logged in the JS console? Just set up logging with:

import xstateNinja, { LogLevels } from 'xstate-ninja'
xstateNinja({ logLevel: LogLevels.debug })

The last "actor updated" log should log a state snapshot which should contain an event. Let's look at it.

rlaffers commented 1 year ago

In the meantime I will release a hotfix which wraps serialization in a try-catch block, so there are no exceptions.

Zehir commented 1 year ago

@Zehir Can you perhaps set up a Codesandbox for this? Or else provide what is logged in the JS console? Just set up logging with:

import xstateNinja, { LogLevels } from 'xstate-ninja'
xstateNinja({ logLevel: LogLevels.debug })

The last "actor updated" log should log a state snapshot which should contain an event. Let's look at it.

You can use my repo; https://github.com/deconz-community/ddf-tools You will need to start both ;

pnpm dev:store
pnpm dev:toolbox

Enable back xstate ninja here : https://github.com/deconz-community/ddf-tools/blob/5a2e8422ab14499fea967ff333b942e667e60c62/packages/toolbox/src/composables/useAppMachine.ts#L160-L166

Then open the page http://localhost:4000/ddf-tools/sandbox

rlaffers commented 1 year ago

Hmm, I cannot reproduce this. The connectToPocketBase service fails (the promise is rejected). I guess that's because I have no pocketBaseUrl in my env.

rlaffers commented 1 year ago

New version with serialization wrapped: https://github.com/rlaffers/xstate-ninja/releases/tag/v1.3.6

Zehir commented 1 year ago

Hmm, I cannot reproduce this. The connectToPocketBase service fails (the promise is rejected). I guess that's because I have no pocketBaseUrl in my env.

Sorry my bad, you can edit the .env file ;

VITE_POCKETBASE_URL="http://127.0.0.1:8090"

I will try to do a reproduction url when I have some time soon thanks

rlaffers commented 1 year ago

Gotcha, I reproduced the problem.

Zehir commented 1 year ago

For that specific case I dont need to see the values inside but it's might not be the case each time

rlaffers commented 1 year ago

So in this case the serialization fails because the context contains some circular data. I will apply the same sanitization function as is used for sanitizing event objects. So most of the data should remain intact, but cirular refs will be replaced with "\". Also stuff like functions, Sets, Maps etc will be stripped because they cannot really be serialized into JSON.

rlaffers commented 1 year ago

Hey @Zehir xstate-ninja@1.3.7 is out. I strongly believe this will fix your issue.

Zehir commented 1 year ago

Sorry but no, I don't understand why but as soon I register with ninja.register(service) I get errors that don't seems related to xstate ninja;

If I disable the chrome extension I don't have any errors

nav-sidebar-level-one.vue:23
Uncaught (in promise) TypeError: app.state.context.machine.gateways.keys is not a function
    at ReactiveEffect.fn (nav-sidebar-level-one.vue:23:5)
    at ReactiveEffect.run (reactivity.esm-bundler.js:178:19)
    at get value [as value] (reactivity.esm-bundler.js:1147:33)
    at unref (reactivity.esm-bundler.js:1026:29)
    at Object.get (reactivity.esm-bundler.js:1032:35)
    at nav-sidebar-level-one.vue:57:20
    at Proxy.renderFnWithContext (runtime-core.esm-bundler.js:766:13)
    at Proxy.render (vue3-perfect-scrollbar.esm.js:102:42)
    at renderComponentRoot (runtime-core.esm-bundler.js:816:16)
    at ReactiveEffect.componentUpdateFn [as fn] (runtime-core.esm-bundler.js:5701:46)
device.ts:52
Uncaught (in promise) TypeError: context.gateway.getDevice is not a function
    at fetchData (device.ts:52:43)
    at Interpreter2._exec (interpreter.js:265:57)
    at Interpreter2.exec (interpreter.js:1022:10)
    at Interpreter2.execute (interpreter.js:383:14)
    at Interpreter2.update (interpreter.js:411:12)
    at interpreter.js:671:13
    at Scheduler2.process (scheduler.js:65:7)
    at Scheduler2.initialize (scheduler.js:28:12)
    at Interpreter2.start (interpreter.js:670:20)
    at Interpreter2.spawnMachine (interpreter.js:1109:8)
Zehir commented 1 year ago

I added some debug, my function stored in the context was replaced by a string;

  services: {
    fetchData: (context) => {
      console.log('fetchData', typeof context.gateway.getDevice)
      console.log('fetchData', context.gateway.getDevice)
      return context.gateway.getDevice({
        params: {
          deviceUniqueID: context.id,
        },
      })
    },
  },
device.ts:53 fetchData string
device.ts:54 fetchData <function>: (t) => this.request(…
device.ts:55 Uncaught (in promise) TypeError: context.gateway.getDevice is not a function
    at fetchData (device.ts:55:30)
    at Interpreter2._exec (interpreter.js:265:57)
    at Interpreter2.exec (interpreter.js:1022:10)
    at Interpreter2.execute (interpreter.js:383:14)
    at Interpreter2.update (interpreter.js:411:12)
    at interpreter.js:671:13
    at Scheduler2.process (scheduler.js:65:7)
    at Scheduler2.initialize (scheduler.js:28:12)
    at Interpreter2.start (interpreter.js:670:20)
    at Interpreter2.spawnMachine (interpreter.js:1109:8)
rlaffers commented 1 year ago

@Zehir This issue was caused by incorrect serialization of context data in xstate-ninja (it did some mutation on the original object). Can you please test your code with version 1.3.9?

Zehir commented 1 year ago

thanks @rlaffers it's worked :)