Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/expose-snapshot-header.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@core/sync-service": patch
---

Include the `electric-snapshot` response header in `access-control-expose-headers` so that cross-origin browser clients can read it.
5 changes: 5 additions & 0 deletions .changeset/quiet-actors-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@electric-sql/client": patch
---

Fix duplicate download of subset snapshot responses caused by the chunk prefetcher mistaking them for a normal chunk to prefetch.
4 changes: 3 additions & 1 deletion packages/sync-service/lib/electric/shapes/api/response.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ defmodule Electric.Shapes.Api.Response do
@electric_schema_header "electric-schema"
@electric_up_to_date_header "electric-up-to-date"
@electric_has_data_header "electric-has-data"
@electric_snapshot_header "electric-snapshot"
@electric_known_error_header "electric-internal-known-error"
@retry_after_header "retry-after"

Expand All @@ -23,6 +24,7 @@ defmodule Electric.Shapes.Api.Response do
@electric_offset_header,
@electric_schema_header,
@electric_up_to_date_header,
@electric_snapshot_header,
@electric_known_error_header,
@retry_after_header
]
Expand Down Expand Up @@ -207,7 +209,7 @@ defmodule Electric.Shapes.Api.Response do
defp put_resp_headers(conn, %__MODULE__{response_type: :subset} = response) do
conn
|> put_cache_header("cache-control", "no-cache", response.api)
|> Plug.Conn.put_resp_header("electric-snapshot", "true")
|> Plug.Conn.put_resp_header(@electric_snapshot_header, "true")
|> put_shape_handle_header(response)
|> put_schema_header(response)
|> put_offset_header(response)
Expand Down
1 change: 1 addition & 0 deletions packages/typescript-client/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export const SHAPE_HANDLE_HEADER = `electric-handle`
export const CHUNK_LAST_OFFSET_HEADER = `electric-offset`
export const SHAPE_SCHEMA_HEADER = `electric-schema`
export const CHUNK_UP_TO_DATE_HEADER = `electric-up-to-date`
export const SNAPSHOT_HEADER = `electric-snapshot`
export const COLUMNS_QUERY_PARAM = `columns`
export const LIVE_CACHE_BUSTER_QUERY_PARAM = `cursor`
export const EXPIRED_HANDLE_QUERY_PARAM = `expired_handle`
Expand Down
6 changes: 5 additions & 1 deletion packages/typescript-client/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
SHAPE_SCHEMA_HEADER,
SHAPE_HANDLE_HEADER,
SHAPE_HANDLE_QUERY_PARAM,
SNAPSHOT_HEADER,
SUBSET_PARAM_LIMIT,
SUBSET_PARAM_OFFSET,
SUBSET_PARAM_ORDER_BY,
Expand Down Expand Up @@ -475,10 +476,13 @@ function getNextChunkUrl(url: string, res: Response): string | void {
const shapeHandle = res.headers.get(SHAPE_HANDLE_HEADER)
const lastOffset = res.headers.get(CHUNK_LAST_OFFSET_HEADER)
const isUpToDate = res.headers.has(CHUNK_UP_TO_DATE_HEADER)
const isSnapshot = res.headers.get(SNAPSHOT_HEADER) === `true`
Comment thread
alco marked this conversation as resolved.

// only prefetch if shape handle and offset for next chunk are available, and
// response is not already up-to-date
if (!shapeHandle || !lastOffset || isUpToDate) return
// snapshot (subset) responses are one-shot and must never be prefetched:
// there is no "next chunk" URL for them, only a repeat of the same request
if (!shapeHandle || !lastOffset || isUpToDate || isSnapshot) return

const nextUrl = new URL(url)

Expand Down
51 changes: 50 additions & 1 deletion packages/typescript-client/test/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import {
createFetchWithConsumedMessages,
parseRetryAfterHeader,
} from '../src/fetch'
import { CHUNK_LAST_OFFSET_HEADER, SHAPE_HANDLE_HEADER } from '../src/constants'
import {
CHUNK_LAST_OFFSET_HEADER,
SHAPE_HANDLE_HEADER,
SNAPSHOT_HEADER,
} from '../src/constants'

describe(`createFetchWithBackoff`, () => {
const initialDelay = 10
Expand Down Expand Up @@ -392,6 +396,51 @@ describe(`createFetchWithChunkBuffer`, () => {
)
})

it(`should not prefetch a snapshot (subset) GET response`, async () => {
const fetchWrapper = createFetchWithChunkBuffer(mockFetch)
const snapshotUrl = `https://example.com/v1/shape?table=foo&subset__limit=50`
const snapshotResponse = new Response(`[]`, {
status: 200,
headers: responseHeaders({
[SHAPE_HANDLE_HEADER]: `123`,
[CHUNK_LAST_OFFSET_HEADER]: `456`,
[SNAPSHOT_HEADER]: `true`,
}),
})

mockFetch.mockResolvedValueOnce(snapshotResponse)

const result = await fetchWrapper(snapshotUrl)
await sleep()

expect(result).toBe(snapshotResponse)
// Snapshot responses must not trigger a speculative prefetch of the
// same subset filters — the client only ever issues one such request.
expect(mockFetch).toHaveBeenCalledTimes(1)
})

it(`should prefetch when the snapshot header is not true`, async () => {
const fetchWrapper = createFetchWithChunkBuffer(mockFetch)
const initialResponse = new Response(`initial chunk`, {
status: 200,
headers: responseHeaders({
[SHAPE_HANDLE_HEADER]: `123`,
[CHUNK_LAST_OFFSET_HEADER]: `456`,
[SNAPSHOT_HEADER]: `false`,
}),
})
const nextResponse = new Response(`next chunk`, { status: 200 })

mockFetch.mockResolvedValueOnce(initialResponse)
mockFetch.mockResolvedValueOnce(nextResponse)

const result = await fetchWrapper(baseUrl)

expect(result).toBe(initialResponse)
const nextUrl = sortUrlParams(`${baseUrl}&handle=123&offset=456`)
expect(mockFetch).toHaveBeenCalledWith(nextUrl, expect.anything())
})

it(`should not prefetch for non-GET requests`, async () => {
const fetchWrapper = createFetchWithChunkBuffer(mockFetch)
const postResponse = new Response(`snapshot chunk`, {
Expand Down
Loading