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

Improved errors thrown when parsing filters #49

Merged
merged 26 commits into from
Jun 15, 2021
Merged

Improved errors thrown when parsing filters #49

merged 26 commits into from
Jun 15, 2021

Conversation

scottinet
Copy link
Contributor

@scottinet scottinet commented Jun 3, 2021

Description

Errors thrown by Koncorde when it parses filters have useful and descriptive messages for humans, but not so much for developpers trying to handle them efficiently.

This PR brings an overhaul of how Koncorde's filter parser handles errors, with the following improvements:

  • errors thrown by Koncorde are now KoncordeParseError errors (extending the plain Error class)
  • errors thrown by Koncorde when parsing filters now have a keyword property, explaining what keyword caused the error
  • errors thrown by Koncorde when parsing filtrers now have a path property, giving the full path of errors, which is useful when dealing with complex filters with a composition of or and and operands
  • errors messages generated when parsing filters now are standardized, they all are like this: "<path>": <message>. That way, users can easily know where their errors are

Example:

const { Koncorde } = require('koncorde');

const k = new Koncorde();

k.validate({ or: [ { and: [ { exists: {} } ] } ] });
/* 
Throws this error:

KoncordeParseError: "or.and.exists": expected object to have exactly 1 property, got 0
    at isObject (/home/scottinet/git/koncorde/lib/transform/standardize.js:787:13)
    at Standardizer.exists (/home/scottinet/git/koncorde/lib/transform/standardize.js:125:5)
    at Standardizer.standardize (/home/scottinet/git/koncorde/lib/transform/standardize.js:92:29)
    at /home/scottinet/git/koncorde/lib/transform/standardize.js:722:22
    at Array.map (<anonymous>)
    at Standardizer._standardizeFilterArray (/home/scottinet/git/koncorde/lib/transform/standardize.js:722:8)
    at Standardizer.and (/home/scottinet/git/koncorde/lib/transform/standardize.js:584:17)
    at Standardizer.standardize (/home/scottinet/git/koncorde/lib/transform/standardize.js:92:29) {
  keyword: 'exists',
  path: 'or.and.exists'
*/

@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #49 (7df12bf) into 4-dev (33f35a4) will decrease coverage by 0.13%.
The diff coverage is 99.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##            4-dev      #49      +/-   ##
==========================================
- Coverage   96.89%   96.76%   -0.14%     
==========================================
  Files          33       34       +1     
  Lines         967      958       -9     
  Branches       32       33       +1     
==========================================
- Hits          937      927      -10     
- Misses         29       30       +1     
  Partials        1        1              
Impacted Files Coverage Δ
lib/index.ts 97.33% <ø> (ø)
lib/transform/standardize.js 97.48% <99.27%> (-0.55%) ⬇️
lib/types/KoncordeParseError.ts 100.00% <100.00%> (ø)
lib/util/hash.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33f35a4...7df12bf. Read the comment docs.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jun 3, 2021

This pull request fixes 2 alerts when merging ad30a17 into 6ea7509 - view on LGTM.com

fixed alerts:

  • 1 for Polynomial regular expression used on uncontrolled data
  • 1 for Useless assignment to property

lib/types/KoncordeParseError.ts Outdated Show resolved Hide resolved
@scottinet scottinet merged commit 5d08287 into 4-dev Jun 15, 2021
@scottinet scottinet deleted the better-errors branch June 15, 2021 12:53
@scottinet scottinet mentioned this pull request Jun 16, 2021
scottinet added a commit that referenced this pull request Jun 16, 2021
# [4.0.0](https://github.com/kuzzleio/koncorde/releases/tag/4.0.0) (2021-06-16)


#### Breaking changes

- [ [#50](#50) ] Rename the maxMinterms configuration   ([scottinet](https://github.com/scottinet))
- [ [#45](#45) ] API revision   ([scottinet](https://github.com/scottinet))
- [ [#44](#44) ] Make the API synchronous   ([scottinet](https://github.com/scottinet))

#### New features

- [ [#49](#49) ] Improved errors thrown when parsing filters   ([scottinet](https://github.com/scottinet))
- [ [#46](#46) ] Add typescript interfaces   ([scottinet](https://github.com/scottinet))

#### Enhancements

- [ [#51](#51) ] Hash with SHA256 instead of Murmur3   ([scottinet](https://github.com/scottinet))

#### Optimizations

- [ [#47](#47) ] Change range checking library   ([scottinet](https://github.com/scottinet))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants