Skip to content

Commit

Permalink
fix(autocomplete): handle missing buildInfo well MONGOSH-1792 (#2013)
Browse files Browse the repository at this point in the history
Recent server source changes put the `buildInfo` command behind
authentication. We previously assumed that this information would
always be available (and, as a corollary, that if the information
from `buildInfo` was absent, topology information would also be absent),
and should stop assuming this.

Long-term, it is probably desirable to rely on `buildInfo` as little
as possible – for example, most of our version checks for autocompletion
should probably be wire version checks instead of server release version
checks.

This should address some failures occuring in the latest-alpha CI jobs.
  • Loading branch information
addaleax authored Jun 3, 2024
1 parent af5f31b commit 57945a5
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/autocomplete/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const localAtlas600 = {
};

const noParams = {
topology: () => Topologies.Standalone,
topology: () => undefined,
apiVersionInfo: () => undefined,
connectionInfo: () => undefined,
getCollectionCompletionsForCurrentDb: () => collections,
Expand Down
10 changes: 6 additions & 4 deletions packages/autocomplete/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
type TypeSignatureAttributes = { [key: string]: TypeSignature };

export interface AutocompleteParameters {
topology: () => Topologies;
topology: () => Topologies | undefined;
connectionInfo: () =>
| undefined
| {
Expand Down Expand Up @@ -373,18 +373,20 @@ function filterShellAPI(
+apiVersionInfo.version <= acceptableApiVersions[1];
} else {
const serverVersion = params.connectionInfo()?.server_version;
if (!serverVersion) return true;

const acceptableVersions = completions[c].serverVersions;
isAcceptableVersion =
!acceptableVersions ||
!serverVersion ||
(semver.gte(serverVersion, acceptableVersions[0]) &&
semver.lte(serverVersion, acceptableVersions[1]));
}

const acceptableTopologies = completions[c].topologies;
const topology = params.topology();
const isAcceptableTopology =
!acceptableTopologies || acceptableTopologies.includes(params.topology());
!acceptableTopologies ||
!topology ||
acceptableTopologies.includes(topology);

return isAcceptableVersion && isAcceptableTopology;
});
Expand Down
9 changes: 5 additions & 4 deletions packages/shell-api/src/shell-instance-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export interface ShellCliOptions {
* from the autocompleter implementation.
*/
export interface AutocompleteParameters {
topology: () => Topologies;
topology: () => Topologies | undefined;
apiVersionInfo: () => Required<ServerApi> | undefined;
connectionInfo: () => ConnectionExtraInfo | undefined;
getCollectionCompletionsForCurrentDb: (collName: string) => Promise<string[]>;
Expand Down Expand Up @@ -394,7 +394,8 @@ export default class ShellInstanceState {
topology: () => {
let topology: Topologies;
const topologyDescription = this.currentServiceProvider.getTopology()
?.description as TopologyDescription;
?.description as TopologyDescription | undefined;
if (!topologyDescription) return undefined;
// TODO: once a driver with NODE-3011 is available set type to TopologyType | undefined
const topologyType: string | undefined = topologyDescription?.type;
switch (topologyType) {
Expand All @@ -413,8 +414,8 @@ export default class ShellInstanceState {
// We're connected to a single server, but that doesn't necessarily
// mean that that server isn't part of a replset or sharding setup
// if we're using directConnection=true (which we do by default).
if (topologyDescription.servers.size === 1) {
const [server] = topologyDescription.servers.values();
if (topologyDescription?.servers.size === 1) {
const [server] = topologyDescription?.servers.values();
switch (server.type) {
case 'Mongos':
topology = Topologies.Sharded;
Expand Down

0 comments on commit 57945a5

Please sign in to comment.