feat(NODE-5988): Provide access to raw results doc after MongoServerError #4016
Conversation
| @@ -425,6 +425,10 @@ describe('CRUD spec v1', function () { | |||
| } | |||
| }); | |||
|
|
|||
| // The Node driver does not have a Collection.modifyCollection helper. | |||
| const SKIPPED_TESTS = ['findOneAndUpdate document validation errInfo is accessible']; | |||
There was a problem hiding this comment.
These tests are different from the collection management specs. These tests use the modifyCollection operation to set up the collection for the test, but they're not testing the driver's modifyCollection helper.
I think it might make sense to implement a unified test format helper for modifyCollection that uses runCommandto send a collMod to the server (i.e., a new operation in operations.ts). Then we could unskip this test. We'd still leave the modifyCollection tests alone though because we don't have a driver helper.
what do you think?
(also see https://github.com/mongodb/specifications/pull/1316/files#r994171436)
There was a problem hiding this comment.
Perhaps we could merge these changes and make a separate subtask or even ticket for adding in the modifyCollection changes?
There was a problem hiding this comment.
Sounds good to me - if we file a separate ticket, can we add a TODO(NODE-XXXX) comment here so we don't forget about it?
There was a problem hiding this comment.
Sounds good! Just filed the ticket NODE-5998.
src/error.ts
Outdated
| @@ -200,6 +200,8 @@ export class MongoError extends Error { | |||
| * @category Error | |||
| */ | |||
| export class MongoServerError extends MongoError { | |||
| /** Raw error result document returned by server. */ | |||
| errorResponse?: ErrorDescription; | |||
There was a problem hiding this comment.
Should this be required? We always assign it in the constructor
There was a problem hiding this comment.
Ideally it should be. There were some type errors since we feed MongoError instances into MongoServerError fields in parts of the driver, so I just casted these instances as MongoServerError as a fix.
There was a problem hiding this comment.
Can you file a subtask to fix the type errors this uncovered (tackling it as follow-up is fine since it's a separate issue)? I think it's a bug in the driver's types but we'll need to look to be sure - ideally we'd fix the type issue instead of just casting it away.
We have a number of instances of MongoServerError that are actually invoked with MongoErrors, which worked before because MongoError was assignable to MongoServerError.
There was a problem hiding this comment.
Sure, here's the link to the ticket.
aditi-khare-mongoDB
force-pushed
the
NODE-5988/6.x-raw-results-doc-MongoServerError
branch
from
21d90ad
to
2b2f64c
Compare
baileympearson
deleted the
NODE-5988/6.x-raw-results-doc-MongoServerError
branch
Description
Provide access to raw results doc in MongoServerError.
What is changing?
See release highlight.
Is there new documentation needed for these changes?
Yes, API docs.
What is the motivation for this change?
Provide support for all values returned by server error.
Release Highlight
Add property errorResponse to MongoServerError
The
MongoServerError.errorResponseproperty now contains the raw error document returned by the server.Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript