Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor addition to exception information on failed bulk-load data due to type validation: Mention the failed column and errorneous value #1557

Open
cstim opened this issue Jul 20, 2023 · 1 comment · May be fixed by #1566

Comments

@cstim
Copy link

cstim commented Jul 20, 2023

Is your feature request related to a problem? If so, please give a short summary of the problem and how the feature would resolve it
If I do a bulk-load of many columns and many values, and then one of the values fail validation, the corresponding exception does not mention the concerned column name and neither the erroneous value. This makes it unnecessarily difficult to narrow down the wrong value that failed validation.

Describe the preferred solution
In https://github.com/tediousjs/tedious/blob/master/src/bulk-load.ts#L188 please add the c.name and also the value to the exception object that is returned. For example by simply adding more properties:

error.column = c.name
error.wrongValue = value

The section of the code would then read:

    try {
      value = c.type.validate(value, c.collation);
    } catch (error: any) {
      error.column = c.name
      error.wrongValue = value
      return callback(error);
    }

Of course you are free to add this information to any other of the properties, or even append it to the message. The main point is that these two bits of information should be added to the exception in any form.

Describe alternatives you've considered
hm... the exception here is pretty much exactly the normal error handling. It is simply missing the information about the place where the exception occurred.

@arthurschreiber
Copy link
Collaborator

@mShan0 @MichaelSun90 Can you take this on? I think it would be best to create a new Error subclass (maybe called ParameterValidationError), have it inherit from whatever error class we currently use (that might be TypeError, but I'm not sure), and define the additional properties on that.

@MichaelSun90 MichaelSun90 linked a pull request Aug 4, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants