ticket-bridge / hyper-durable

Simple and useful Durable Object abstraction
MIT License
63 stars 3 forks source link

#<Object> could not be cloned. #27

Open AggressivelyMeows opened 1 year ago

AggressivelyMeows commented 1 year ago

Trying to get a service setup using this lib however im running into some strange issues with persistance.

For some reason, when doing a filter over an array, it breaks with the error "# could not be cloned.". While debugging, it appears that forcing JS to create a new Array using JSON.parse/stringify, it successfully saves. Although i'm not convinced this hotfix will work 100% of the time.

console.log(
  this.repeat_queue, // [   { event: 'testEvent.created', object: {  }, repeat_count: 0 } ]
  this.repeat_queue.isProxy // true
)

this.repeat_queue = this.repeat_queue.filter(e => e.id != event.id)

console.log(
  this.repeat_queue, // [   { event: 'testEvent.created', object: {  }, repeat_count: 0 } ] - Yes, the array is the same. However its considered dirty due to the Filter (??)
  this.repeat_queue.isProxy // true
)

If you wanted to see the code for yourself: https://github.com/drivly/webhooks.do/blob/main/src/webhook.durable.js#L119

AggressivelyMeows commented 1 year ago

Poking to see if anything is being done for this issue?

travismfrank commented 1 year ago

Hi @AggressivelyMeows, apologies for the delay. I'll take a look this weekend 😄

AggressivelyMeows commented 1 year ago

Overwriting the persist function and forcing the durable to do sanity checks fixes the issue. it should also auto-cast all proxies to their JSON format. The code below is what im using to get around the issue for now, however I dont have time to test every possible edge-case. But it should work for 98% of all cases.

  async persist() {
    let newProps = false
    for (let key of this.state.dirty) {
      const v = this.state[key]

      // Skip functions and promises.
      if (typeof v === 'function') continue
      if (typeof v === 'object' && v instanceof Promise) continue
      if (typeof v === 'undefined') continue

      const value = JSON.parse(JSON.stringify(this.state[key]))

      await this.storage.put(key, value)

      if (!this.state.persisted.has(key)) {
        this.state.persisted.add(key)
        newProps = true
      }

      this.state.dirty.delete(key)
    }

    if (newProps) await this.storage.put('persisted', this.state.persisted)
  }
nathanclevenger commented 1 year ago

Thanks @TravisFrankMTG ! @AggressivelyMeows is working for us at Drivly now, and this is a blocking issue on a key feature ... we really appreciate your support here, thank you again!

travismfrank commented 1 year ago

@AggressivelyMeows The auto-cast to and from JSON is a nice patch! It introduces quite a bit of overhead compared to the structured clone algorithm, though, so I looked for a different solution.

Your initial observation that the array is considered dirty even though it's deeply equal to the filtered array is a good starting point. .filter creates a shallow copy of the array. Shallow copies (even identical array literals) don't pass the strict equality check I was using in the hyperProxy (target[key] !== value), resulting in the property marked dirty. I'm now using isEqual from lodash to perform a deep comparison, solving this problem.

I was not able to reproduce the original error you described (in my experience, that's the error thrown by the structured clone algorithm). I briefly thought this might happen when the SCA attempts to clone shallow copies, but that doesn't appear to be the case. If you can reproduce the error and share it, that would be super helpful. 🙏

I released v0.2.0-rc1 with these changes, can you pull that in and comment out your persist & clone hotfixes and let me know if it solves the issue? Thanks!