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

bad string formatting when constructing error - "Cannot convert object to primitive value" #11639

Closed
andreimatei opened this issue Mar 4, 2024 · 22 comments

Comments

@andreimatei
Copy link

andreimatei commented Mar 4, 2024

Issue Description

I think there's a bug on some code path that's trying to construct an error. Looks similar too what was fixed in #11516, but I'm running 3.9.5 which should have that fix.
I see:

index.ts:97 Uncaught (in promise) ApolloError: Cannot convert object to primitive value
    at new ApolloError2 (index.ts:97:5)
    at Object.error (QueryManager.ts:353:48)
    at notifySubscription (module.js:137:18)
    at onNotify (module.js:176:3)
    at SubscriptionObserver2.error (module.js:229:5)
    at asyncMap.ts:45:28

The underlying error returned by the server is the requested element is null which the schema does not allow.

Link to Reproduction

Reproduction Steps

No response

@apollo/client version

3.9.5

@jerelmiller
Copy link
Member

Hey @andreimatei 👋

We have a few errors in the code base that are converted using substitution, so its very possible there is one out there that isn't substituted properly. It's a bit difficult to tell where in the code base this is though without more information. Are you able to figure out what triggered this error? Unfortunately I'm not sure the stack trace has enough information to know where this is triggered from 😕

@andreimatei
Copy link
Author

I can tell you that with the Chrome debugger, I see that the message of the error contains a stack:

TypeError: Cannot convert object to primitive value
    at Array.join (<anonymous>)
    at Array.toString (<anonymous>)
    at String (<anonymous>)
    at warn (<anonymous>)
    at console.overrideMethod [as warn] (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/react_devtools_backend_compact.js:13024:11)
    at http://localhost:5173/node_modules/.vite/deps/chunk-L5PWGWHX.js?v=d913821a:216:21
    at Function.warn (http://localhost:5173/node_modules/.vite/deps/chunk-L5PWGWHX.js?v=d913821a:282:8)
    at warnAboutDataLoss (http://localhost:5173/node_modules/.vite/deps/@apollo_client.js?v=d913821a:3345:45)
    at http://localhost:5173/node_modules/.vite/deps/@apollo_client.js?v=d913821a:3049:13
    at Array.forEach (<anonymous>)

Maybe the warnAboutDataLoss and the Array.toString() in there are enough of a clue about where this is coming from?

@andreimatei
Copy link
Author

The respective line from warnAboutDataLoss is

  globalThis.__DEV__ !== false && invariant.warn(12, fieldName, parentType, childTypenames.length ? "either ensure all objects of type " + childTypenames.join(" and ") + " have an ID or a custom merge function, or " : "", typeDotName, existing, incoming);

@jerelmiller
Copy link
Member

jerelmiller commented Mar 5, 2024

Awesome, that warnAboutDataLoss is useful.

Would you mind turning on dev messages and seeing if perhaps one of those values is not a type we expect?

import { loadErrorMessages, loadDevMessages } from "@apollo/client/dev";

if (__DEV__) {
  loadDevMessages();
  loadErrorMessages();
}

If not, perhaps adding a breakpoint in that code and inspecting the values passed to invariant.warn might help? Apologies, its a bit difficult for me to try this out on my own without a reproduction to follow 😅

@Titozzz
Copy link

Titozzz commented Mar 5, 2024

It happened to us when there was no merge function and the items had no "id" key.
Something like:

{
  __typename: "a",
  id: 1,
  arrayOfSomething : {
    __typename: "b",
    something: "foo",
    anotherthing: "bar",
  },
}

When merging those arrayOfSomething, it would warn if no merge policy if defined. But the warn crashed if the item has no id.
-> To avoid the crash: add an id (if you have one)
-> To fix properly: add a policy

@andreimatei
Copy link
Author

I believe that, for some reason, Vite was using an old version of Apollo Client (different from the version in my package.json / node_modules. I've compared the sources the browser was reporting with what I was seeing on a different machine where the error did not reproduce, and they did not match. Deleting the node_modules/.vite fixed the problem. So I suspect that the problem had been fixed by a recent patch.

Apologies for the noise... Closing.

Copy link
Contributor

github-actions bot commented Mar 6, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@adrienharnay
Copy link

This is not fixed, as @Titozzz said it's still happening.

@jerelmiller jerelmiller reopened this Mar 7, 2024
@jerelmiller
Copy link
Member

@adrienharnay @Titozzz which version of Apollo Client are you using?

@adrienharnay
Copy link

@adrienharnay @Titozzz which version of Apollo Client are you using?

We're on 3.8.7 and plan to update to latest next week (but I don't believe this has been fixed since, though I may be wrong).

@jerelmiller
Copy link
Member

@adrienharnay it does look like we had a fix specifically for the cache override warning in #11483 which was released in 3.8.10. Could you try a quick test by upgrading to 3.8.10 and seeing if the issue goes away?

@jerelmiller
Copy link
Member

Seeing as #11483 landed in 3.8.10 which should solve this issue, I'm going to go ahead and close this. If anyone upgrades to 3.8.10 or greater and still sees this issue, please add a comment with a reproduction and I'd be happy to reopen. Thanks!

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@Netail
Copy link

Netail commented Apr 5, 2024

Hi @jerelmiller I am still facing this issue with 3.9.9. It currently throws an error and breaks caching entirely

Seems to be related to hidden attributes inside of the incoming/existing objects

@phryneas
Copy link
Member

phryneas commented Apr 9, 2024

@Netail I'm trying to reproduce your situation here, but I'm not really getting anywhere.

Do you maybe have some script in place in your application or a browser extension that wraps console.warn and causes this?

What happens if you execute console.warn("This should display an object %o nicely embedded", { embedded: true }) in your browser console while you're on your page?
(If that works, what happens if you do that somewhere in your app?)

For reference, this is what I'd expect it to look like:
image

@Netail
Copy link

Netail commented Apr 9, 2024

@phryneas the custom console log works both in the browser and when I add it manually in before the other log in node_modules/@apolloc/client. And does not seem to be related to browser extensions

@phryneas
Copy link
Member

phryneas commented Apr 9, 2024

@Netail huh, very weird. But that's valuable feedback, too. Thanks! I'll keep digging

@Netail
Copy link

Netail commented Apr 9, 2024

@Netail huh, very weird. But that's valuable feedback, too. Thanks! I'll keep digging

I'll try to set up a demo, if I somehow able to reproduce it

@phryneas
Copy link
Member

@Netail I've done my best to reproduce your error, but I cannot.

Could you take a look at this CodeSandbox and report back if you are getting the error in any scenario?

Load the Sandbox first:

https://codesandbox.io/p/sandbox/trigger-cache-warning-cltdvn

Then visit the standalone Sandbox:

https://cltdvn.csb.app/

Scenarios to check:

  • Browser window with DevTools enabled - you should get three warnings, the first a link and the two other "spelled out"
  • Browser window with DevTools disabled - you should get three warnings, the first two should be links and the last one "spelled out"

@prsauer
Copy link

prsauer commented Apr 26, 2024

We've been seeing these for a number of array queries in our platform as well -- I'm having a hard time reproducing in a sandbox but there is some issue with objects that don't like having .join called on them

@psamim
Copy link
Contributor

psamim commented May 6, 2024

I am seeing it too. In my case, I used Chrome debugger and the error comes from here:

https://github.com/apollographql/apollo-client/blob/main/src/cache/inmemory/writeToStore.ts#L853

I have the error because both existing and incoming objects do not have .toString method. In the Chrome debugger, I saved incoming object as temp1 on the window and these are my observations:

console.log('%o', temp1)
> Uncaught TypeError: Cannot convert object to primitive value
console.log('%o', {...temp1} )
> {__typename: 'Configuration', totalNumber: 10, updatedAt: '2024-05-06T11:10:11.684236649Z' }
temp1.toString()
> Uncaught TypeError: temp1.toString is not a function

This change fixes it in my case. I created a PR.

main...psamim:apollo-client:patch-2

Copy link
Contributor

github-actions bot commented Jun 6, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants