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

Converting circular structure to JSON #195

Open
2 tasks done
palmtown opened this issue Oct 16, 2023 · 12 comments
Open
2 tasks done

Converting circular structure to JSON #195

palmtown opened this issue Oct 16, 2023 · 12 comments

Comments

@palmtown
Copy link
Contributor

palmtown commented Oct 16, 2023

Steps to reproduce

(First please check that this issue is not already solved as described
here
)

  • Tell us what broke. The more detailed the better.
  • If you can, please create a simple example that reproduces the issue and link to a gist, jsbin, repo, etc.

Uncaught error at https://github.com/feathersjs-ecosystem/feathers-sync/blob/release/lib/core.js#L62 that causes feathersjs to return a 500 error although the request completed successfully, with exception that the sync failed. In short, the mongoose object is now set at context.arguments[1].mongoose and triggers this error as it contains circular structures.

Expected behavior

No errors converting object to JSON.

Actual behavior

Tell us what happens instead

Error is generated at https://github.com/feathersjs-ecosystem/feathers-sync/blob/release/lib/core.js#L62 due to context.arguments[1].mongoose.

System configuration

Tell us about the applicable parts of your setup.

Module versions (especially the part that's not working): 3.0.3

NodeJS version: 18

Operating System: Ubuntu

Browser Version:

React Native Version:

Module Loader:

@daffl
Copy link
Member

daffl commented Oct 16, 2023

If you payload contains circular JSON you can use a custom serializer

@palmtown
Copy link
Contributor Author

Hello @daffl

Thanks for providing the custom serializer solution; I can use that to solve my problem. Nonetheless, are you aware that the mongoose object is now set in context.argument[1].mongoose which includes circular structures? Note that this error was not triggering previously. Also, I am not setting this in my code.

Also, if newer versions now set context.argument[1].mongoose, do you believe a fix to feathers-sync would be the appropiate resolution vs. having developers implement a customer serializer?

Again, thanks for the recommended solution, I will use that.

@daffl
Copy link
Member

daffl commented Oct 17, 2023

Did you pass the Mongoose object somewhere? Unless explicitly done so, I don't believe there should be circular objects in the context.

@palmtown
Copy link
Contributor Author

Hello @daffl

Good question! I do not believe I have done that. However, I'm going to review my code and commits to double check and be sure. As you may know, the context.argument[1] object seems to be variables set by feathers such as query, headers, provider etc. whereas context.argument[0] contains the data that I set.

I'll dig a little deeper and see what I can find out.

@jordandenison
Copy link

@daffl I am getting this error as well when trying to authenticate with a custom auth strategy. Happens for both authentication.create and users.patch. I'm not sure how to or if using a custom serializer would work in this case but putting delete context.params.resolve right before the emit in core.js seems to fix the issue. Do you have any idea why this might be happening, and do you forsee any issues with removing this property before emitting?

@daffl
Copy link
Member

daffl commented Feb 20, 2024

Hm, yes, that might indeed have to be excluded.

@jordantelus
Copy link

Rut roh. Any suggestions? I have also tried delete context.params.resolve.originalContext but the issue still occurs.

@daffl
Copy link
Member

daffl commented Feb 20, 2024

Serializing it without the property should work:

app.configure(
  sync({
    uri: 'redis://localhost:6379',
    serialize: (payload) => {
      const { context } = payload
      const { resolve, ...params } = context.params

      return JSON.stringify({
        ...payload,
        context: {
          ...context,
          params
        }
      })
    }
  })
);

@jordantelus
Copy link

Hmm, that code still gives the error yet this does not:

    serialize: (payload) => {
      delete payload.context.params.resolve
      return JSON.stringify(payload)
    }

Which does not make a whole lot of sense. But knowing that it is safe to exclude the resolve property is good to know. Thank you!

@daffl
Copy link
Member

daffl commented Feb 20, 2024

That does seem strange since it should really just do the same thing.

@jordantelus
Copy link

Yes. Exactly. I am super puzzled as well. lol

@pedobry
Copy link

pedobry commented Apr 4, 2024

There is also resolve once more in the context.arguments[1]. What works for me:

  serialize: (payload) => {
      // use custom serializer to remove context.params.resolve and
      // context.arguments from the payload
      // otherwise, it will throw an error when trying to serialize the payload
      // due to circular structure
      const { context } = payload;
      const { resolve, ...params } = context.params;

      return JSON.stringify({
          ...payload,
          context: {
              ...context,
              arguments: [],
              params,
          },
      });
  },

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

5 participants