electric-sql / electric

Sync little subsets of your Postgres data into local apps and services.
https://electric-sql.com
Apache License 2.0
6.47k stars 156 forks source link

chore (ts-client): validate required Electric headers on response #1957

Closed kevin-dp closed 3 days ago

kevin-dp commented 2 weeks ago

Fixes https://github.com/electric-sql/electric/issues/1950

Questions:

thruflo commented 2 weeks ago

Can we also validate an electric-cursor header when we receive a 204 in response to a live request?

Not having the cursor param can result in re-requesting to the same URL.

thruflo commented 2 weeks ago

Plus can we please write a section in the https://electric-sql.com/docs/guides/troubleshooting guide about this and include a link to it in the error message?

Basically we would want to explain that if headers are missing then your proxy needs to make sure it includes them, maybe with a code sample showing how (Kyle has some proxy code that clones the headers).

I can write this if you'd like.

KyleAMathews commented 2 weeks ago

Checking just 200s for headers seems fine as it covers all the headers.

netlify[bot] commented 1 week ago

Deploy Preview for electric-next ready!

Name Link
Latest commit a75084096429f0145b07c087b9bb8cc59ce2f5a5
Latest deploy log https://app.netlify.com/sites/electric-next/deploys/6735e2e7c3e38e0008963aaa
Deploy Preview https://deploy-preview-1957--electric-next.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

kevin-dp commented 1 week ago

@KyleAMathews Can you have another review at this PR to see if you approve the changes? There are some required headers that are checked on all 2xx responses. There are also some headers that are required only in live mode and some only in non-live mode. I also check those on 2xx responses.

I will open a separate docs PR before merging this one.

kevin-dp commented 3 days ago

For reference, this is the accompanying docs PR: https://github.com/electric-sql/electric/pull/1976