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

Handle non-array errors gracefully #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jacobpgn
Copy link

If results is not an array here, something has probably gone really wrong.

In production we're typically seeing this when some part of the world between our GraphQL server and client blows up and returns an HTML error page. Spreading results when it's a string creates really unusable errors like this:

GraphQL Error: {
  "0": "\n",
  "1": "<",
  "2": "!",
  "3": "d",
  "4": "o",
  "5": "c",
  "6": "t",
  "7": "y",
  "8": "p",
  "9": "e",
  "10": " ",
  "11": "h",
  "12": "t",
  "13": "m",
  "14": "l",
  "15": ">",
  "16": "\n",
  "17": "<",
  "18": "h",
  "19": "t",
  "20": "m",
  "21": "l",
  "22": ">",
  "23": "\n",
  "24": "<",
  "25": "h",
  "26": "e",
  "27": "a",
  "28": "d",
  "29": ">",
  "30": "\n",
  "31": " ",
  "32": " ",
  "33": " ",
  "34": " ",
  "35": "<",
  "36": "t",
  "37": "i",
  "38": "t",
  "39": "l",
  "40": "e",
  "41": ">",
  "42": "E",
  "43": "x",
  "44": "a",
  "45": "m",
  "46": "p",
  "47": "l",
  "48": "e",
  "49": " ",
  "50": "D",
  "51": "o",
  "52": "m",
  "53": "a",
  "54": "i",
  "55": "n",
  "56": "<",
  "57": "/",
  "58": "t",
  "59": "i",
  "60": "t",
  "61": "l",
  "62": "e",
  "63": ">",
  "64": "\n",
  "65": "\n",
  "66": " ",
  "67": " ",
  "68": " ",
  "69": " ",

... snipped!

  "1252": "/",
  "1253": "h",
  "1254": "t",
  "1255": "m",
  "1256": "l",
  "1257": ">",
  "1258": "\n",
  "status": 502
}

With this change it'd instead be the slightly-more-debuggable:

Invalid response: \n<!doctype html>\n<html>\n<head>\n    <title>Example Domain</title>\n\n    <meta charset=\"utf-8\" />\n    <meta http-equiv=\"Content-type\" content=\"text/html; charset=utf-8\" />\n    <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\" />\n    <style type=\"text/css\">\n    body {\n        background-color: #f0f0f2;\n        margin: 0;\n        padding: 0;\n        font-family: \"Open Sans\", \"Helvetica Neue\", Helvetica, Arial, sans-serif;\n\n    }\n    div {\n        width: 600px;\n        margin: 5em auto;\n        padding: 50px;\n        background-color: #fff;\n        border-radius: 1em;\n    }\n    a:link, a:visited {\n        color: #38488f;\n        text-decoration: none;\n    }\n    @media (max-width: 700px) {\n        body {\n            background-color: #fff;\n        }\n        div {\n            width: auto;\n            margin: 0 auto;\n            border-radius: 0;\n            padding: 1em;\n        }\n    }\n    </style>\n</head>\n\n<body>\n<div>\n    <h1>Example Domain</h1>\n    <p>This domain is established to be used for illustrative examples in documents. You may use this\n    domain in examples without prior coordination or asking for permission.</p>\n    <p><a href=\"http://www.iana.org/domains/example\">More information...</a></p>\n</div>\n</body>\n</html>\n

@timsuchanek 👋 hi! Do you think it'd be suitable to get this in and bump prisma-binding?

@Gdewilde
Copy link

Hi @jacobpgn, how did you resolve this?

@jacobpgn
Copy link
Author

Hey @Gdewilde! Sorry, I'm not using this package any more so I don't have a good solution to share.

@Gdewilde
Copy link

Gdewilde commented Apr 5, 2021

No worries! Thanks for the update

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

Successfully merging this pull request may close these issues.

2 participants