apollo-server: Throwing circular object in resolver breaks apollo-server (500 HTTP error)
OK, so the title isn’t 100% accurate. Putting this code in a resolver:
await axios.zeit.post('/v2/domains/salamander.ai/records', {
name: subdomain,
type: 'A',
value: instanceUrl
})
Lead to this error when the request failed (printed in console & returned to client):
TypeError: Converting circular structure to JSON
at JSON.stringify (<anonymous>)
at prettyJSONStringify (/home/ashton/code/6labs/packages/salamander-apollo/node_modules/apollo-server-core/dist/runHttpQuery.js:19:17)
at Object.<anonymous> (/home/ashton/code/6labs/packages/salamander-apollo/node_modules/apollo-server-core/dist/runHttpQuery.js:297:33)
at Generator.next (<anonymous>)
at fulfilled (/home/ashton/code/6labs/packages/salamander-apollo/node_modules/apollo-server-core/dist/runHttpQuery.js:4:58)
at process._tickCallback (internal/process/next_tick.js:68:7)
I wanted to simplify a bit so came up with this, but it worked fine (good error with message === "[Object object]"
):
const a = {}
const b = {a}
a.b = b
throw a
Not sure why the first thing is bad but second is OK.
FYI I’m using apollo-server-express@2.0.0-rc.5
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 8
- Comments: 22 (4 by maintainers)
Workaround: use the
formatResponse
option function to catch the circular reference, re-serialize with a safe stringify function, before handing off to apollo:@twelve17 Object serialization can be complicated, but the burden of determining the correct serialization for an object is best defined by the implementation that generated the object.
Contrary to your claim above, Apollo Server’s use of
JSON.stringify
on an error object does not mean that Apollo Server has an opinion on how to serialize an object with cycles in it, merely that we have an object that we would like to be converted into a JSON string.In fact, it would be somewhat odd if we took an opinionated approach on that particular serialization concern since
JSON.stringify
itself already defers to an object’stoJSON
property (if it’s defined as a function) to allow exactly this sort of customization by the object creator (rather than leaning on other implementations which need to serialize it).And of course, there’s more than one way to handle cycles!
So, what does a
toJSON
implementation look like, in practice?:This allows an object to declare exactly how it should be serialized, which is even more relevant for a project like Axios who has consciously chosen to create cycles in their error objects.
A cursory search shows that this certainly isn’t a new problem for Axios users, as is demonstrated by the existence of https://github.com/axios/axios/issues/836 and https://github.com/axios/axios/issues/1301. As recently as the end of last year they accepted https://github.com/axios/axios/pull/1625, which defines a
toJSON
function on their errors, in an attempt to resolve this precise problem!Now, if Apollo Server’s use of
JSON.stringify
to serialize errors into a JSON string is just straight up not obeying that object’stoJSON
implementation, then that seems like something worth investigating further, but that’s different than taking an opinionated stance on how cycles should be serialized in anError
object.Of course, luckily, there isn’t much to debug in Apollo Server since we just call
JSON.stringify
:https://github.com/apollographql/apollo-server/blob/d8ade4df4d343cd2ad441ebcb88c87a7f986dde6/packages/apollo-server-core/src/runHttpQuery.ts#L441-L443
As to the solution here? It looks like the fix in that PR (https://github.com/axios/axios/pull/1625) was released into
axios@0.19.0
as of2019-05-30T16:13:16.930Z
. Sadly, that’s was just a few days after the reproduction provided above by @mdlavin in https://github.com/apollographql/apollo-server/issues/1433#issuecomment-494368741 (which, tangentially, to be honest, is a very nice and clean reproduction! Thank you!) was made.While slightly better timing here would have maybe avoided this frustation entirely, I’m happy to say that merely updating that reproduction to use
axios@0.19.0
withnpm install axios@0.19.0
, solves the problem and changes its output from:Before updating
axios
(Withaxios@0.19.0
)After updating
axios
(to0.19.0
)@abernix Thank you so much for the detailed explanation. I had forgotten about the delegation to each object’s “.toJSON” call. I appreciate your insight and I stand corrected. Also, thanks for the references to Axios’s fix!
@jbaxleyiii I can see in principle that the implementation should not have an opinion on serialization, but the error from the OP is not that the serialization is not matching a certain opinion, it is that it is failing to serialise at all, and thus the actual error is not being returned. Apollo Server using
JSON.serialize
means it already has an opinion on how circular references should be serialised. Using something like safe serialize could avoid those issues and still allow implementors’ use offormatResponse
to treat circular responses in a different way.After a lot of thinking about this we don’t think that Apollo Server having its own serialization and protection around this is the right approach. In fact, the spec isn’t specific about serialization (https://graphql.github.io/graphql-spec/draft/#sec-Serialization-Format) meaning custom serialization may treat circular responses differently
For people hitting this bug due to Apollo + AWS X-Ray, I put a public Gist up at https://gist.github.com/mdlavin/4e7dffd5786341cb807f9add897b26fa with my workaround
I ran into this last week and it felt like fixing it in Apollo was a more scalable approach than having every consuming project adopt the workaround. I hope the fix is appropriate
just came across this error. Shouldn’t it be handle by apollo itself ?