Skip to content

Commit

Permalink
fix(shell-api): improve error message on reading from secondary via r…
Browse files Browse the repository at this point in the history
…unCommand MONGOSH-1679 (#1805)

* fix: improve error message on reading from secondary via runCommand

* test: add tests

* Update packages/shell-api/src/mongo-errors.ts

Co-authored-by: Anna Henningsen <[email protected]>

* Update packages/shell-api/src/database.ts

Co-authored-by: Anna Henningsen <[email protected]>

* explicit type

* adjust all messages

---------

Co-authored-by: Anna Henningsen <[email protected]>
  • Loading branch information
paula-stacho and addaleax authored Jan 31, 2024
1 parent 968fe96 commit a846579
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 7 deletions.
17 changes: 17 additions & 0 deletions packages/shell-api/src/database.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
MongoshUnimplementedError,
} from '@mongosh/errors';
import type { ClientEncryption } from './field-level-encryption';
import type { MongoServerError } from 'mongodb';

chai.use(sinonChai);

describe('Database', function () {
Expand Down Expand Up @@ -323,6 +325,21 @@ describe('Database', function () {
}
);
});

it('rephrases the "NotPrimaryNoSecondaryOk" error', async function () {
const originalError: Partial<MongoServerError> = {
message: 'old message',
codeName: 'NotPrimaryNoSecondaryOk',
code: 13435,
};
serviceProvider.runCommandWithCheck.rejects(originalError);
const caughtError = await database
.runCommand({ someCommand: 'someCollection' })
.catch((e) => e);
expect(caughtError.message).to.contain(
'e.g. db.runCommand({ command }, { readPreference: "secondaryPreferred" })'
);
});
});

describe('adminCommand', function () {
Expand Down
16 changes: 12 additions & 4 deletions packages/shell-api/src/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,19 @@ export default class Database extends ShellApiWithMongoClass {
cmd = { [cmd]: 1 };
}

const hiddenCommands = new RegExp(HIDDEN_COMMANDS);
if (!Object.keys(cmd).some((k) => hiddenCommands.test(k))) {
this._emitDatabaseApiCall('runCommand', { cmd, options });
try {
const hiddenCommands = new RegExp(HIDDEN_COMMANDS);
if (!Object.keys(cmd).some((k) => hiddenCommands.test(k))) {
this._emitDatabaseApiCall('runCommand', { cmd, options });
}
return await this._runCommand(cmd, options);
} catch (error: any) {
if (error.codeName === 'NotPrimaryNoSecondaryOk') {
const message = `not primary - consider passing the readPreference option e.g. db.runCommand({ command }, { readPreference: "secondaryPreferred" })`;
(error as Error).message = message;
}
throw error;
}
return this._runCommand(cmd, options);
}

/**
Expand Down
13 changes: 12 additions & 1 deletion packages/shell-api/src/mongo-errors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ describe('mongo-errors', function () {
expect(r.code).to.equal(13435);
expect(r.message).to.contain('setReadPref');
});

it('does not rephrase a NotPrimaryNoSecondaryOk error with db.runCommand example', function () {
const e = new MongoError(
'not primary - consider passing the readPreference option e.g. db.runCommand({ command }, { readPreference: "secondaryPreferred" })'
);
e.code = 13435;
const r = rephraseMongoError(e);
expect(r).to.equal(e);
expect(r.code).to.equal(13435);
expect(r.message).not.to.contain('setReadPref');
});
});
});

Expand Down Expand Up @@ -101,7 +112,7 @@ describe('mongo-errors', function () {
expect.fail('expected error');
} catch (e: any) {
expect(e).to.equal(error);
expect(e.message).to.contain('not primary and secondaryOk=false');
expect(e.message).to.contain('not primary');
expect(e.message).to.contain('db.getMongo().setReadPref()');
expect(e.message).to.contain('readPreference');
}
Expand Down
6 changes: 4 additions & 2 deletions packages/shell-api/src/mongo-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ const ERROR_REPHRASES: MongoErrorRephrase[] = [
{
// NotPrimaryNoSecondaryOk (also used for old terminology)
code: 13435,
replacement:
'not primary and secondaryOk=false - consider using db.getMongo().setReadPref() or readPreference in the connection string',
replacement: (message) =>
message.includes('db.runCommand')
? message
: 'not primary - consider using db.getMongo().setReadPref() or readPreference in the connection string',
},
];

Expand Down

0 comments on commit a846579

Please sign in to comment.