Uploaded image for project: 'Node.js Driver'
  1. Node.js Driver
  2. NODE-5484

Add Node16+ support for the `cause` property in errors

    • Type: Icon: Task Task
    • Resolution: Fixed
    • Priority: Icon: Unknown Unknown
    • 6.0.0
    • Affects Version/s: None
    • Component/s: None
    • 3
    • 2
    • Needed
    • Hide

      Review the ticket description for general accuracy and completeness

      • Bug - Confirm that the bug still exists
      • Task / Feature / Improvement - Ensure every section of the template is filled out and makes sense
      • Build failure - Investigate and confirm the cause of the build failure
      • Spec change - Check whether any more recent changes have been made to the spec that might affect the implementation requirements

      What is the expected behavior?

      • What do the official driver or server docs currently say about this functionality?
        • What should they say?
          • If revisions or additions are needed, mark the ticket as docs changes needed and fill out the doc changes form
      • What do our api or readme docs currently say about this functionality?
        • They expose our MongoError constructors publicly
        • What should they say?
          • They should not publicly expose MongoError constructors
        • Capture any revisions or additions in the ticket documentation AC
      • If applicable, what does the common drivers spec say? (Note: your kickoff partner should independently review the spec)
        • Are any clarifications or revisions needed?
      • If applicable, what do other drivers do?
        • If there is no common spec, is a common spec needed?
      • What should the behavior be?
        • MongoError and subclass constructors should accept cause option and use call to super to set the MongoError.cause field instead of doing so manually
        • MongoError.cause for base and subclasses should be an Error
        • MongoError and subclass constructors should be internal
        • MongoCryptError and subclasses should be subclasses of MongoError
      • Update the ticket description and implementation requirements as needed
        • Done

      Review and address any unknowns explicitly called out in the ticket

      What will be the impact on users?

      • Who will be impacted?
        • Users who manually construct MongoErrors, but this should not be many users
      • Why might users care about this change?
      • Capture relevant detail in the "User Impact" section of the ticket description
        • Done

      What will be the impact on any downstream projects? (e.g., shell, mongoose)

      • Only used in shell tests
      • Update follow up requirements and create subtasks for follow up or coordination actions
        • Done

      What variables affect the feature in question?

      • Server versions
      • Deployment types
      • Auth settings
      • Server and client configuration options
      • Specific apis / api options
      • Runtime or bundler settings
      • Special sequences of operations
      • Any other special conditions

      How should all the identified variables be tested?

      • Identify happy path and error case combinations of variables
        • Given [variables], when [action is performed], [feature] should [behave in the expected way]
      • How will we achieve the necessary coverage for these cases?
        • Automated spec tests?
          • Are there test runner changes required?
          • How up to date are our current tests and runners?
        • New integration or prose tests?
        • Unit tests?
          • yes
      • Will we need to modify any existing tests?
        • Possibly
      • Is there technical debt that will affect the implementation of new or existing tests?
        • No
      • Do we have the necessary tooling infrastructure already in place for any new tests?
        • Yes
      • Update test requirements on the ticket to reflect reality
        • Done
      • Create subtasks for any testing groundwork that can happen independently of the implementation

      What is the scope of the code changes?

      • List the code bases and the areas of each code base that will need changes
        • Node v6 driver. Should just be src/error.ts and src/client-side-encryption/errors.ts
      • Is there technical debt in any of these areas that will affect the implementation?
        • No
      • Identify any existing adjacent functionality that could be impacted by these changes
        • Is there sufficient existing test coverage for the adjacent functionality?
          • Update ticket test AC and create subtask(s) to cover existing functionality if coverage is missing
      • If multiple libraries are affected, determine the order in which changes need to go in
      • Create subtasks for the implementation (at least one per affected codebase)

      What is the expected impact on performance?

      • Do we have existing performance coverage for the affected areas?
      • Do we need to add new coverage?
        • No
        • Update ticket test AC and create subtask(s) as needed

      Consider backport requirements

      • Should this be backported?
        • No, breaking
      • What would be the cost of a backport?

      Is the metadata of this ticket accurate and complete?

      • Double check the acceptance criteria to ensure it accurately captures the expected behavior, test, and follow-up requirements
        • Done
      • Double check the documentation requirements
        • Done
      • Double check the task breakdown to ensure it covers all actionable items in the ticket AC
        • Done
      Show
      Review the ticket description for general accuracy and completeness Bug - Confirm that the bug still exists Task / Feature / Improvement - Ensure every section of the template is filled out and makes sense Build failure - Investigate and confirm the cause of the build failure Spec change - Check whether any more recent changes have been made to the spec that might affect the implementation requirements What is the expected behavior? What do the official driver or server docs currently say about this functionality? What should they say? If revisions or additions are needed, mark the ticket as docs changes needed and fill out the doc changes form What do our api or readme docs currently say about this functionality? They expose our MongoError constructors publicly What should they say? They should not publicly expose MongoError constructors Capture any revisions or additions in the ticket documentation AC If applicable, what does the common drivers spec say? (Note: your kickoff partner should independently review the spec) Are any clarifications or revisions needed? If applicable, what do other drivers do? If there is no common spec, is a common spec needed? What should the behavior be? MongoError and subclass constructors should accept cause option and use call to super to set the MongoError.cause field instead of doing so manually MongoError.cause for base and subclasses should be an Error MongoError and subclass constructors should be internal MongoCryptError and subclasses should be subclasses of MongoError Update the ticket description and implementation requirements as needed Done Review and address any unknowns explicitly called out in the ticket What will be the impact on users? Who will be impacted? Users who manually construct MongoErrors, but this should not be many users Why might users care about this change? Capture relevant detail in the "User Impact" section of the ticket description Done What will be the impact on any downstream projects? (e.g., shell, mongoose) Only used in shell tests Update follow up requirements and create subtasks for follow up or coordination actions Done What variables affect the feature in question? Server versions Deployment types Auth settings Server and client configuration options Specific apis / api options Runtime or bundler settings Special sequences of operations Any other special conditions How should all the identified variables be tested? Identify happy path and error case combinations of variables Given [variables] , when [action is performed] , [feature] should [behave in the expected way] How will we achieve the necessary coverage for these cases? Automated spec tests? Are there test runner changes required? How up to date are our current tests and runners? New integration or prose tests? Unit tests? yes Will we need to modify any existing tests? Possibly Is there technical debt that will affect the implementation of new or existing tests? No Do we have the necessary tooling infrastructure already in place for any new tests? Yes Update test requirements on the ticket to reflect reality Done Create subtasks for any testing groundwork that can happen independently of the implementation What is the scope of the code changes? List the code bases and the areas of each code base that will need changes Node v6 driver. Should just be src/error.ts and src/client-side-encryption/errors.ts Is there technical debt in any of these areas that will affect the implementation? No Identify any existing adjacent functionality that could be impacted by these changes Is there sufficient existing test coverage for the adjacent functionality? Update ticket test AC and create subtask(s) to cover existing functionality if coverage is missing If multiple libraries are affected, determine the order in which changes need to go in Create subtasks for the implementation (at least one per affected codebase) What is the expected impact on performance? Do we have existing performance coverage for the affected areas? Do we need to add new coverage? No Update ticket test AC and create subtask(s) as needed Consider backport requirements Should this be backported? No, breaking What would be the cost of a backport? Is the metadata of this ticket accurate and complete? Double check the acceptance criteria to ensure it accurately captures the expected behavior, test, and follow-up requirements Done Double check the documentation requirements Done Double check the task breakdown to ensure it covers all actionable items in the ticket AC Done
    • Not Needed
    • Hide
      1. What would you like to communicate to the user about this feature?
        • MongoCryptErrors are now subclasses of MongoError
        • constructors for MongoError and subclasses are now private
      2. Would you like the user to see examples of the syntax and/or executable code and its output?
        • No
      3. Which versions of the driver/connector does this apply to?
        • driver >= v6
      Show
      What would you like to communicate to the user about this feature? MongoCryptErrors are now subclasses of MongoError constructors for MongoError and subclasses are now private Would you like the user to see examples of the syntax and/or executable code and its output? No Which versions of the driver/connector does this apply to? driver >= v6

      Use Case

      As a driver engineer,
      I want our errors to properly support the `cause` property
      So that we can provide actionable errors to users.

      User Impact

      Should be minimal - users shouldn't be constructing MongoErrors themselves.

      Dependencies

      • n/a

      Unknowns

      • Should we take this opportunity to mark the constructors of our errors internal?
        • Yes
        • No, this will outright remove documentation from the constructors, but users will still be allowed to construct them. Rather, we should leave the constructors public but add tsdoc comments that explain that they are not intended for users and are not covered under semver

      Acceptance Criteria

      Implementation Requirements

      • Adjust the constructor of `MongoError` to `constructor(message: string, options: { cause?: Error })` (this matches the official Error constructor)
        • Adjust subclasses of MongoError to properly call `super` with the required parameters.
      • Narrow the type of the `cause` property to an `Error` for our drivers' errors.
      • Remove Node14 logic to attach the cause to MongoError manually in the constructor and remove the `cause` property from the MongoError class.
      • Make the constructors of all our errors internal (see open question).
      • Update doc comments on Error constructors (see open question)
      • Make `MongoCryptError` a subclass of MongoError
        • This was deferred until the constructor of `MongoError` properly supports a cause.

      Testing Requirements

      • Add tests asserting that each MongoCryptError is a subclass of MongoError.

      Documentation Requirements

      • Update `etc/notes/Errors.md` with new errors
      • Add release note mentioning that MongoCryptError is now subclassing MongoError
      • Add release note mentioning that constructors will be made private
      • Ensure that the constructors of our errors are public but the API documentation explicitly states that they are not for external use.

      Follow Up Requirements

      • File a ticket to revisit how we document and expose classes which have internal constructors

            Assignee:
            warren.james@mongodb.com Warren James
            Reporter:
            bailey.pearson@mongodb.com Bailey Pearson
            Bailey Pearson
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: