Skip to content

Commit

Permalink
Mark fully rolled out properties as required in the sync protocol (#3…
Browse files Browse the repository at this point in the history
…4538)

I believe `baseVersion` should always be sent now (introduced in get-convex/convex#11884 which is before Convex 1.0), and `queryJournal` should always be present (but can be `null`).

This slightly simplifies the types + removes tests for behavior that shouldn't really happen anymore.

There's code that indicates that you could hit an `AuthError` while there is no `version`, but I think it's impossible to hit that code path (based on auditing the spots where `ErrorMetadata::unauthenticated` is used), but I'll clean this up separately.

GitOrigin-RevId: da540fffd9185b95924f40e635e3b1299346d55f
  • Loading branch information
sshader authored and Convex, Inc. committed Feb 26, 2025
1 parent 3bded4b commit dca8234
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 25 deletions.
15 changes: 5 additions & 10 deletions npm-packages/convex/src/browser/sync/authentication_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,19 +186,14 @@ export class AuthenticationManager {
const { baseVersion } = serverMessage;
// Versioned AuthErrors are ignored if the client advanced to
// a newer auth identity
if (baseVersion !== null && baseVersion !== undefined) {
// Error are reporting the previous version, since the server
// didn't advance, hence `+ 1`.
if (!this.syncState.isCurrentOrNewerAuthVersion(baseVersion + 1)) {
this._logVerbose("ignoring auth error for previous auth attempt");
return;
}
void this.tryToReauthenticate(serverMessage);
// Error are reporting the previous version, since the server
// didn't advance, hence `+ 1`.
if (!this.syncState.isCurrentOrNewerAuthVersion(baseVersion + 1)) {
this._logVerbose("ignoring auth error for previous auth attempt");
return;
}

// TODO: Remove after all AuthErrors are versioned
void this.tryToReauthenticate(serverMessage);
return;
}

// This is similar to `refetchToken` defined below, in fact we
Expand Down
1 change: 1 addition & 0 deletions npm-packages/convex/src/browser/sync/client_node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ test("Tests can encode longs in server messages", () => {
queryId: 0,
value: 0.0,
logLines: ["[LOG] 'Got stuff'"],
journal: null,
},
],
};
Expand Down
2 changes: 2 additions & 0 deletions npm-packages/convex/src/browser/sync/local_state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ test("has outstanding state older than restart", () => {
queryId: queryId1,
value: "hi",
logLines: [],
journal: null,
},
],
});
Expand All @@ -69,6 +70,7 @@ test("has outstanding state older than restart", () => {
queryId: queryId2,
value: "hi",
logLines: [],
journal: null,
},
],
});
Expand Down
8 changes: 3 additions & 5 deletions npm-packages/convex/src/browser/sync/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,17 +234,15 @@ type StateModification =
queryId: QueryId;
value: JSONValue;
logLines: LogLines;
// Optional because old backend versions don't send this.
journal?: QueryJournal;
journal: QueryJournal;
}
| {
type: "QueryFailed";
queryId: QueryId;
errorMessage: string;
logLines: LogLines;
errorData: JSONValue;
// Optional because old backend versions don't send this.
journal?: QueryJournal;
journal: QueryJournal;
}
| {
type: "QueryRemoved";
Expand Down Expand Up @@ -294,7 +292,7 @@ export type ActionResponse = ActionSuccess | ActionFailed;
export type AuthError = {
type: "AuthError";
error: string;
baseVersion?: IdentityVersion;
baseVersion: IdentityVersion;
};
type FatalError = {
type: "FatalError";
Expand Down
14 changes: 4 additions & 10 deletions npm-packages/convex/src/react/auth_websocket.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,12 @@ describe.sequential.skip("auth websocket tests", () => {
});

// This happens when a user opens a page after their cached token expired
test("Reauthenticate after token expiration without versioning", async () => {
await testRauthenticationOnInvalidTokenSucceeds(undefined);
});
test("Reauthenticate after token expiration with versioning", async () => {
await testRauthenticationOnInvalidTokenSucceeds(0);
});

async function testRauthenticationOnInvalidTokenSucceeds(
authErrorBaseVersion: number | undefined,
authErrorBaseVersion: number,
) {
await withInMemoryWebSocket(async ({ address, receive, send, close }) => {
const client = testReactClient(address);
Expand Down Expand Up @@ -180,16 +177,11 @@ describe.sequential.skip("auth websocket tests", () => {
});

// This is usually a misconfigured server rejecting any token
test("Fail when tokens are always rejected without versioning", async () => {
await testRauthenticationFails(undefined);
});
test("Fail when tokens are always rejected with versioning", async () => {
await testRauthenticationFails(0);
});

async function testRauthenticationFails(
authErrorBaseVersion: number | undefined,
) {
async function testRauthenticationFails(authErrorBaseVersion: number) {
await withInMemoryWebSocket(async ({ address, receive, send, close }) => {
const client = testReactClient(address);

Expand Down Expand Up @@ -228,6 +220,7 @@ describe.sequential.skip("auth websocket tests", () => {
send({
type: "AuthError",
error: AUTH_ERROR_MESSAGE,
baseVersion: authErrorBaseVersion,
});
close();

Expand Down Expand Up @@ -406,6 +399,7 @@ describe.sequential.skip("auth websocket tests", () => {
queryId: 0,
value: 42,
logLines: [],
journal: null,
},
],
});
Expand Down
1 change: 1 addition & 0 deletions npm-packages/convex/src/react/react_node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ function transition(): ServerMessage {
queryId: 0,
value: 0.0,
logLines: [],
journal: null,
},
],
};
Expand Down

0 comments on commit dca8234

Please sign in to comment.