Add multiple error reporting #192

vladfrangu · 2020-11-24T10:11:01Z

This PR closes #5

What's been added

This PR adds multiple validation error reporting to Arguments and AnyPredicates, by attaching a validationErrors map to the resulting error (heavily simplified). I am open to discussion about possible alternatives, issues, concerns, or just how you would want to see this implemented instead. Personally, I believe this implementation is clean enough (at least from the UX side of things) that it shouldn't be too breaking (however it does classify as semver major). With the current implementation, you may then take the resulting errors and present them however you may want in your API for example. If the full error message is desired to show all errors, let me know and I will look into how that could be displayed in a nicer way

The error stacktrace is also kept, currently looking like this by running the example.js file in the root of the repository

r [ArgumentError]: Multiple errors were encountered. Please check the `validationErrors` property of the thrown error
    at fn (C:\Users\Vlad\Desktop\ow\example.js:13:2)
    at C:\Users\Vlad\Desktop\ow\example.js:17:2
    at logError (C:\Users\Vlad\Desktop\ow\example.js:6:3)
    at Object.<anonymous> (C:\Users\Vlad\Desktop\ow\example.js:16:1)
    at Module._compile (internal/modules/cjs/loader.js:1076:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:941:32)
    at Function.Module._load (internal/modules/cjs/loader.js:782:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) {
  validationErrors: Map(1) {
    'input' => [
      'Expected `input` to be of type `string` but received type `number`',
      'Expected string `input` to have a minimum length of `10`, got `10`'
    ]
  }
}

In-depth, going file by file

argument-error.ts
- stack trace is now kept, as well as the error name. (and, while after running npm run build it will prepend that random letter for the name (can be fixed by not mangling the class names iirc), it will also output the name itself, as shown above)
- Made the stacktrace for browsers not lose the message or name
  - testing is required for this for any regressions
- holds a Map<string, string[]> representing any validation errors
index.ts
- Generates the stack that is passed down, usually just for browsers or envs without captureStackTrace
predicates/any.ts
- now generates a different message if all of the passed predicates error out
- ignores any non-ArgumentError exceptions (although... do we want that? Discussion needed)
predicates/predicate.ts
- stores all errors predicates return in a map, generates different message if multiple errors are thrown

IssueHunt Summary

Referenced issues

This pull request has been submitted to:

#5: Report multiple type errors

test/test.ts

source/utils/generate-stack.ts

sindresorhus · 2020-11-27T13:22:08Z

This change will need a lot more tests.

source/argument-error.ts

vladfrangu · 2020-12-02T11:23:38Z

This change will need a lot more tests.

@sindresorhus I think these tests should cover almost everything that is needed! Of course, however, if you have any other edge cases you may want me to test, please let me know! 😋

I tried to cover every code path possible, and, as coverages report, all are covered

vladfrangu · 2020-12-20T23:32:23Z

I merged master into this PR, let me know if there's anything you'd like changed 🙏

source/utils/generate-stack.ts

sindresorhus · 2020-12-26T10:26:38Z

If the full error message is desired to show all errors, let me know and I will look into how that could be displayed in a nicer way

Per the original issue. The error is meant for humans, so it should present all the errors by default.

source/predicates/any.ts

sindresorhus · 2020-12-26T10:28:50Z

Sorry for the slow reply. I'm just finally now getting through my long list of notifications.

vladfrangu · 2020-12-26T10:30:33Z

Sorry for the slow reply. I'm just finally now getting through my long list of notifications.

Don't worry, I would've assumed that was the case! It's totally understandable

vladfrangu · 2020-12-26T17:20:43Z

There we go @sindresorhus, the errors outputted are now in one big message, while also keeping the validationErrors property public so users can present it any way they wish

ArgumentError: Expected `input` to be of type `string` but received type `number`
Expected string `input` to have a minimum length of `10`, got `10`
Expected string `input` to end with `owo`, got `10`
    at ow (C:\Users\Vlad\Desktop\ow\dist\index.js:21:23)
    at fn (C:\Users\Vlad\Desktop\ow\example.js:13:2)
    at C:\Users\Vlad\Desktop\ow\example.js:17:2
    at logError (C:\Users\Vlad\Desktop\ow\example.js:6:3)
    at Object.<anonymous> (C:\Users\Vlad\Desktop\ow\example.js:16:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) {
  validationErrors: Map(1) {
    'input' => [
      'Expected `input` to be of type `string` but received type `number`',
      'Expected string `input` to have a minimum length of `10`, got `10`',
      'Expected string `input` to end with `owo`, got `10`'
    ]
  }
}

source/utils/generate-argument-error-message.ts

sindresorhus · 2021-01-07T08:03:20Z

Seems like the code coverage is below the threshold.

vladfrangu · 2021-01-07T13:50:03Z

Seems like the code coverage is below the threshold.

~~That's..confusing to say the least.. I can't even see why on codecov, and on local npm run test, I get this~~

Local Coverage report

-------------------------------------|---------|----------|---------|---------|-------------------
File                                 | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------------------------|---------|----------|---------|---------|-------------------
All files                            |   99.48 |    94.27 |   93.03 |   99.46 |                   
 source                              |   99.28 |    94.74 |   59.49 |   99.26 | 
  argument-error.ts                  |     100 |    66.67 |     100 |     100 | 9
  index.ts                           |     100 |      100 |   30.43 |     100 | 
  modifiers.ts                       |     100 |      100 |     100 |     100 | 
  predicates.ts                      |   98.55 |      100 |   68.63 |   98.55 | 303
  test.ts                            |     100 |      100 |     100 |     100 | 
 source/operators                    |     100 |      100 |     100 |     100 | 
  not.ts                             |     100 |      100 |     100 |     100 | 
 source/predicates                   |     100 |    98.28 |     100 |     100 | 
  any.ts                             |     100 |      100 |     100 |     100 | 
  array-buffer.ts                    |     100 |      100 |     100 |     100 | 
  array.ts                           |     100 |      100 |     100 |     100 | 
  base-predicate.ts                  |     100 |      100 |     100 |     100 | 
  boolean.ts                         |     100 |      100 |     100 |     100 | 
  data-view.ts                       |     100 |      100 |     100 |     100 | 
  date.ts                            |     100 |      100 |     100 |     100 | 
  error.ts                           |     100 |      100 |     100 |     100 | 
  map.ts                             |     100 |      100 |     100 |     100 | 
  number.ts                          |     100 |      100 |     100 |     100 | 
  object.ts                          |     100 |      100 |     100 |     100 |
  predicate.ts                       |     100 |    96.97 |     100 |     100 | 87
  set.ts                             |     100 |      100 |     100 |     100 |
  string.ts                          |     100 |      100 |     100 |     100 |
  typed-array.ts                     |     100 |      100 |     100 |     100 |
  weak-map.ts                        |     100 |      100 |     100 |     100 |
  weak-set.ts                        |     100 |      100 |     100 |     100 |
 source/utils                        |   97.66 |    92.86 |     100 |    97.5 |
  generate-argument-error-message.ts |     100 |      100 |     100 |     100 |
  generate-stack.ts                  |     100 |      100 |     100 |     100 |
  has-items.ts                       |     100 |      100 |     100 |     100 |
  infer-label.ts                     |   90.32 |       92 |     100 |      90 | 25,33,41
  is-valid-identifier.ts             |     100 |      100 |     100 |     100 |
  match-shape.ts                     |     100 |    86.36 |     100 |     100 | 55-58,91
  of-type-deep.ts                    |     100 |      100 |     100 |     100 |
  of-type.ts                         |     100 |      100 |     100 |     100 |
  random-id.ts                       |     100 |      100 |     100 |     100 |
 source/utils/node                   |     100 |       75 |     100 |     100 |
  is-node.ts                         |     100 |       75 |     100 |     100 | 1
-------------------------------------|---------|----------|---------|---------|-------------------

~~A lot of these aren't even caused by me... Maybe I should try a rebase?~~

src: throw error on any predicate only if there are any chore: make default message empty Doesn't matter much as it's overwritten immediately after chore: drop redundant nullish coalescing fix: declare the validationErrors prop as readonly Gives a TS compilation error if you try to reassign it chore: generate error message only if there are errors chore: fix bug that caused validators to error if validator func throws Noticeable with a function like ```js ow( null, ow.any( ow.string.minLength(12), ow.string.includes('owo') ) ); ``` chore: apply code style suggestion Co-authored-by: Sindre Sorhus <[email protected]> chore: fix stack generation and comment chore: move from t.assert to t.is src: add more tests to increase coverage chore: correct test messages chore: fix rebase

vladfrangu · 2021-01-07T14:20:33Z

@sindresorhus back again, after a rebase (technically it was squash, commit, rebase, commit, commit, squash, commit), seems like everything is back in order! Probably my local branch was outdated compared to your master and caused codecov to not report right. 👍

sindresorhus · 2021-01-08T17:22:01Z

Nice work. Thanks ;)

sindresorhus requested changes Nov 27, 2020

View reviewed changes

test/test.ts Outdated Show resolved Hide resolved

source/utils/generate-stack.ts Outdated Show resolved Hide resolved

sindresorhus reviewed Nov 27, 2020

View reviewed changes

source/argument-error.ts Outdated Show resolved Hide resolved

vladfrangu requested a review from sindresorhus December 7, 2020 09:58

sindresorhus reviewed Dec 26, 2020

View reviewed changes

source/utils/generate-stack.ts Outdated Show resolved Hide resolved

sindresorhus changed the title ~~feat: add multiple error reporting~~ Add multiple error reporting Dec 26, 2020

sindresorhus reviewed Dec 26, 2020

View reviewed changes

source/predicates/any.ts Outdated Show resolved Hide resolved

sindresorhus requested changes Jan 7, 2021

View reviewed changes

source/utils/generate-argument-error-message.ts Outdated Show resolved Hide resolved

vladfrangu force-pushed the feat/predicates/multiple-error-reporting branch 2 times, most recently from d5ff153 to bb3be97 Compare January 7, 2021 14:06

vladfrangu force-pushed the feat/predicates/multiple-error-reporting branch from a69888f to 1a91170 Compare January 7, 2021 14:15

vladfrangu requested a review from sindresorhus January 7, 2021 14:20

sindresorhus merged commit f467ee9 into sindresorhus:master Jan 8, 2021

karlhorky mentioned this pull request May 24, 2022

Show all possible errors exhaustively (option?) #237

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multiple error reporting #192

Add multiple error reporting #192

vladfrangu commented Nov 24, 2020 •

edited

Loading

sindresorhus commented Nov 27, 2020

vladfrangu commented Dec 2, 2020 •

edited

Loading

vladfrangu commented Dec 20, 2020

sindresorhus commented Dec 26, 2020

sindresorhus commented Dec 26, 2020

vladfrangu commented Dec 26, 2020

vladfrangu commented Dec 26, 2020

sindresorhus commented Jan 7, 2021

vladfrangu commented Jan 7, 2021 •

edited

Loading

vladfrangu commented Jan 7, 2021

sindresorhus commented Jan 8, 2021

Add multiple error reporting #192

Add multiple error reporting #192

Conversation

vladfrangu commented Nov 24, 2020 • edited Loading

What's been added

In-depth, going file by file

Referenced issues

sindresorhus commented Nov 27, 2020

vladfrangu commented Dec 2, 2020 • edited Loading

vladfrangu commented Dec 20, 2020

sindresorhus commented Dec 26, 2020

sindresorhus commented Dec 26, 2020

vladfrangu commented Dec 26, 2020

vladfrangu commented Dec 26, 2020

sindresorhus commented Jan 7, 2021

vladfrangu commented Jan 7, 2021 • edited Loading

vladfrangu commented Jan 7, 2021

sindresorhus commented Jan 8, 2021

vladfrangu commented Nov 24, 2020 •

edited

Loading

vladfrangu commented Dec 2, 2020 •

edited

Loading

vladfrangu commented Jan 7, 2021 •

edited

Loading