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

Suggestion: store.subscribe should throw if syntax is invalid #1

Closed
deslee opened this issue Jan 6, 2021 · 2 comments
Closed

Suggestion: store.subscribe should throw if syntax is invalid #1

deslee opened this issue Jan 6, 2021 · 2 comments

Comments

@deslee
Copy link

deslee commented Jan 6, 2021

Referencing sanity-io/groq-js#29

Was fighting a bug for a few hours that was caused by groq-store swallowing an error caused by passing an invalid query to groq-js's parse() function. This invalid query is likely a bug in groq-js because the query works in every other sanity-io tool.

When groq-store is consumed by another library, the error thrown by groq-js is not logged in the console, so it was very difficult to trace it to this.

Therefore, I think the store.subscribe() function should check to see if the query is valid, because otherwise the subscription would appear as though it is broken / not receiving any updates.

@judofyr
Copy link
Contributor

judofyr commented Jan 15, 2021

I can't really reproduce it here:

store.subscribe(`*[wat`, {}, (err, result) => {
  if (err) {
    console.error('Oh no, an error:', err)
    return
  }

  console.log('Result:', result)
})

This logs:

Oh no, an error: GroqSyntaxError: Syntax error in GROQ query at position 5
    at Object.parse$1 [as parse] (/Users/holm/Documents/Sanity/groq-store/node_modules/groq-js/dist/groq-js.cjs.development.js:5427:38)
    at query (/Users/holm/Documents/Sanity/groq-store/dist/node/groq-store.js:314:25)
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  position: 5
}
Oh no, an error: GroqSyntaxError: Syntax error in GROQ query at position 5
    at Object.parse$1 [as parse] (/Users/holm/Documents/Sanity/groq-store/node_modules/groq-js/dist/groq-js.cjs.development.js:5427:38)
    at query (/Users/holm/Documents/Sanity/groq-store/dist/node/groq-store.js:314:25)
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  position: 5
}

It's a bit weird that it logs it twice, but other than that it's getting sent correctly.

@deslee
Copy link
Author

deslee commented Jan 17, 2021

My mistake, I incorrectly assumed the error was not being sent back up, but it was. I wasn't checking the error object when using the hook in next-sanity

@deslee deslee closed this as completed Jan 17, 2021
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

2 participants