Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Errors handling enhancement #212

Merged
merged 34 commits into from
Feb 6, 2023
Merged

Errors handling enhancement #212

merged 34 commits into from
Feb 6, 2023

Conversation

gmourier
Copy link
Member

@gmourier gmourier commented Jan 4, 2023

🤖 API Diff Put the link of the GitHub comment generated by bump.sh if generated; Apply the OpenApi label n/a


Summary

meilisearch/meilisearch#3246

  • Replace bad_request linked to a property/parameter with dedicated error codes
  • Replace generic missing_parameter with dedicated error codes
  • Rename some error codes to follow a predictable naming convention.
  • Move no_space_left_on_device coming from the internal type to a newly introduced system type
    • Add too_many_open_files error code to system
    • Add io_error error code to system
  • Replace immutable_field with dedicated error codes (used for the /keys API and /indexes API on the PATCH method)
  • CATCH-UP spec bad_request variant when an unknown field is given
  • Add invalid_task_index_uids error code
  • Remove old mentions of "this :placeholder is inferred when the message is generated"

Changes

code property

Swap API

POST /swap-indexes

  • duplicate_index_foundinvalid_swap_duplicate_index_found ✅ [RENAME]
  • Sending an indexes array not containing exactly 2 indexUids for a swap operation object returns a bad_request error -> invalid_swap_indexes ✅ [NEW]
  • Sending an invalid index uid in the indexes array returns an invalid_swap_indexes error ✅
  • Omitting the indexes array in a swap payload returns a missing_swap_indexes error ✅ [NEW]

Index API

GET /indexes

  • limit parameter error bad_requestinvalid_index_limit ✅ [NEW]
  • offset parameter error bad_requestinvalid_index_offset ✅ [NEW]

GET /indexes/:indexUid

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid error ✅

POST /indexes

  • bad_request when uid is missing in the payload → missing_index_uid ✅ [NEW]
  • Sending a value with a different type than string or null for primaryKey will return a bad_request error → invalid_index_primary_key ✅ [NEW]

PATCH /indexes/:indexUid

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid error ✅
  • Sending a value with a different type than string or null for primaryKey will return a bad_request error → invalid_index_primary_key ✅ [NEW]
  • Modifying the uid property returns an immutable_index_uid error ✅ [NEW]
  • Modifying the createdAt property returns an immutable_index_created_at error ✅ [NEW]
  • Modifying the updatedAt property returns an immutable_index_updated_at error ✅ [NEW]

DELETE /indexes/:indexUid

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid error ✅

Documents API

GET /indexes/:indexUid/documents

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid
  • fields parameter error bad_requestinvalid_document_fields ✅ [NEW]
  • limit parameter error bad_requestinvalid_document_limit ✅ [NEW]
  • offset parameter error bad_requestinvalid_document_offset ✅ [NEW]

GET /indexes/:indexUid/documents/:documentId

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid
  • fields parameter error bad_requestinvalid_document_fields ✅ [NEW]

POST/PUT /indexes/:indexUid/documents

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid
  • ?primaryKey parameter error bad_requestinvalid_index_primary_key ✅ [NEW]

DELETE /indexes/:indexUid/documents

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid

DELETE /indexes/:indexUid/documents/:documentId

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid

POST /indexes/:indexUid/documents/delete-batch

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid

Search API ✅

GET/POST /indexes/:indexUid/search

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid

bad_request replaced by specific error codes per parameter👇

  • invalid_search_q [NEW]
  • invalid_search_offset [NEW]
  • invalid_search_limit [NEW]
  • invalid_search_page [NEW]
  • invalid_search_hits_per_page [NEW]
  • invalid_search_attributes_to_retrieve [NEW]
  • invalid_search_attributes_to_crop [NEW]
  • invalid_search_crop_length [NEW]
  • invalid_search_attributes_to_highlight [NEW]
  • invalid_search_show_matches_position [NEW]
  • invalid_filter -> invalid_search_filter [RENAME]
  • invalid_sort -> invalid_search_sort [RENAME]
  • invalid_search_facets [NEW]
  • invalid_search_highlight_pre_tag [NEW]
  • invalid_search_highlight_post_tag [NEW]
  • invalid_search_crop_marker [NEW]
  • invalid_search_matching_strategy [NEW]

Settings API ✅

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid

bad_request replaced by specific error codes per parameter/object👇**

  • invalid_settings_displayed_attributes [NEW]
  • invalid_settings_searchable_attributes [NEW]
  • invalid_filter -> invalid_settings_filterable_attributes [RENAME]
  • invalid_sort ->invalid_settings_sortable_attributes [RENAME]
  • invalid_settings_ranking_rules [NEW]
  • invalid_settings_stop_words [NEW]
  • invalid_settings_synonyms [NEW]
  • invalid_settings_distinct_attribute [NEW]
  • invalid_settings_typo_tolerance [NEW]
    • invalid_typo_tolerance_min_word_size_for_typos (Merged with invalid_settings_typo_tolerance as a message variant☝️)
  • invalid_settings_faceting [NEW]
  • invalid_settings_pagination [NEW]

Task API ✅

GET /tasks, POST /tasks/cancel, DELETE /tasks (Filters)

  • invalid_task_date_filter
    • invalid_task_before_enqueued_at [NEW]
    • invalid_task_after_enqueued_at [NEW]
    • invalid_task_before_started_at [NEW]
    • invalid_task_after_started_at [NEW]
    • invalid_task_before_finished_at [NEW]
    • invalid_task_after_finished_at [NEW]
  • invalid_task_uids_filter -> invalid_task_uids ✅ [RENAME]
  • invalid_task_types_filter -> invalid_task_types ✅ [RENAME]
  • invalid_task_statuses_filter -> invalid_task_statuses ✅ [RENAME]
  • invalid_task_canceled_by_filter -> invalid_task_canceled_by ✅ [RENAME]
  • invalid_task_index_uids ✅ [NEW]
  • limit parameter bad_requestinvalid_task_limit ✅ [NEW] (Only for GET)
  • from parameter bad_requestinvalid_task_from ✅ [NEW] (Only for GET)

Keys API

POST /keys

  • missing_parameter
    • missing_api_key_actions ✅ [NEW]
    • missing_api_key_indexes ✅ [NEW]
    • missing_api_key_expires_at ✅ [NEW]

PATCH /keys/:key_or_uid

  • immutable_field
    • immutable_api_Key_uid ✅ [NEW]
    • immutable_api_key_actions ✅ [NEW]
    • immutable_api_key_indexes ✅ [NEW]
    • immutable_api_key_expires_at ✅ [NEW]
    • immutable_api_key_created_at ✅ [NEW]
    • immutable_api_key_update_at ✅ [NEW]

GET /keys

  • limit parameter bad_requestinvalid_api_key_limit ✅ [NEW]
  • offset parameter bad_requestinvalid_api_key_offset ✅ [NEW]

Stats API

GET /indexes/:indexUid/stats

  • Sending an invalid index uid in the :indexUid path parameter returns an invalid_index_uid

Misc

  • invalid_geo_fieldinvalid_document_geo_field ✅ [RENAME]
  • dump_not_found (Catch-up) [REMOVED]

type property

system ✅ [NEW]

  • Moved no_space_left_on_device from internal to system ✅ [UPDATED]
  • Add io_error error code ✅ [NEW]
  • Add too_many_open_files error code ✅ [NEW]

internal

  • Remove index_not_accessible [REMOVED]

Out Of Scope

n/a


Attention To Reviewers

I hope to have captured all the changes that can be made to the generic errors; please pay special attention to ensure we don't miss any.

Please do not review the message field. We will do a DX pass in the future. Changing the message field content is not considered breaking.

The OpenAPI spec never had full compliance with all our error codes; it will be added in another PR in the future.


Misc

  • [] Update OpenAPI specification file (if needed; Apply the OpenApi label) n/a
  • [] Update telemetry datapoints (if needed; Apply the Telemetry label) n/a

@gmourier gmourier added v1.0.0 In Progress Feature specification is in elaboration. Important changes can still occurs in the specification. ⚠️ Breaking Introduce a breaking change. Q1:2023 labels Jan 4, 2023
@gmourier gmourier mentioned this pull request Jan 4, 2023
1 task
text/0055-sort.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
Copy link
Member Author

@gmourier gmourier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sharing comments with you @irevoire (Also signaling to @Kerollmops and @curquiza for an update)

I've used //TODO marker in the spec for now for the following reasons:

  1. Marking message field of the newly introduced error codes (We should re-use the previous message when it was a bad_request and generated by serde to move fast, I think)
  2. I've marked the 2 asynchronous errors that we wanted to make synchronous initially

@gmourier gmourier changed the title 🚧 Errors enhancement 🚧 Errors handling enhancement Jan 4, 2023
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gmourier I read the spec and suggested some changes. Feel free to ignore them if you want to!

text/0059-geo-search.md Outdated Show resolved Hide resolved
text/0060-tasks-api.md Outdated Show resolved Hide resolved
text/0118-search-api.md Show resolved Hide resolved
text/0061-error-format-and-definitions.md Outdated Show resolved Hide resolved
text/0061-error-format-and-definitions.md Outdated Show resolved Hide resolved
text/0061-error-format-and-definitions.md Outdated Show resolved Hide resolved
text/0061-error-format-and-definitions.md Show resolved Hide resolved
text/0061-error-format-and-definitions.md Outdated Show resolved Hide resolved
text/0061-error-format-and-definitions.md Show resolved Hide resolved
@gmourier gmourier added Ready For Review Feature specification must be reviewed. and removed In Progress Feature specification is in elaboration. Important changes can still occurs in the specification. labels Jan 9, 2023
@bidoubiwa
Copy link
Contributor

bidoubiwa commented Jan 10, 2023

The following codes seem to be removed based on the spec; my bad if some are not.
The ones that are ✅, are mentioned in the body of this PR.

  • invalid_geo_field
  • invalid_filter
  • invalid_sort
  • invalid_task_uids_filter
  • invalid_task_statuses_filter
  • invalid_task_types_filter
  • invalid_task_canceled_by_filter
  • invalid_task_date_filter
  • missing_parameter
  • immutable_field
  • api_key_already_exists
  • invalid_api_key_uid
  • invalid_api_key_name
  • invalid_api_key_description
  • invalid_api_key_actions
  • invalid_api_key_indexes
  • invalid_api_key_expires_at
  • invalid_typo_tolerance_min_word_size_for_typos
  • index_already_exists
  • `index_primary_key_already_exists
  • invalid_index_uid
  • primary_key_inference_failed
  • missing_document_id
  • invalid_document_id
  • invalid_ranking_rule
  • invalid_geo_field
  • duplicate_index_found
  • dump_not_found
  • task_not_found
  • invalid_store_file

Questions:

  • is bad_request still a possible code?

@irevoire
Copy link
Member

irevoire commented Jan 10, 2023

is bad_request still a possible code?

Yes, every time we can’t link the error to a specific field.
e.g. A syntax error happened, you provided an unknown field, maybe more? 🤔

meili-bors bot added a commit to meilisearch/meilisearch-js that referenced this pull request Feb 1, 2023
1438: Update error codes for Meilisearch v1.0.0 r=bidoubiwa a=bidoubiwa

See [specification](meilisearch/specifications#212)

Co-authored-by: Charlotte Vermandel <charlottevermandel@gmail.com>
@gmourier gmourier merged commit aa12826 into release-v1.0.0 Feb 6, 2023
@gmourier gmourier deleted the error-handler-revamp branch February 6, 2023 10:17
gmourier added a commit that referenced this pull request Feb 6, 2023
* Bump open-api spec

* Simplify dump version section since v1.0 can import all dumps (#203)

* Simplify dump version section since v1.0 can import all dumps

* Relax target import version

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>

---------

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>

* Rename dump command from `--dumps-dir` to `--dump-dir` (#204)

* Propagate the change to --dumps-dir to the specification

* Rename section header in openAPI documentation

* Dump destination -> Dump directory

* Finish destination -> directory renaming

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

---------

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Update the specification of soft-deleted documents (#206)

* Grammarly'd soft-deletion page

* Soft-deleted: update the functional specification

* Remove `--max-index-size` and `--max-task-db-size` (#207)

* Robustify Primary Key Inference - Update related errors (#208)

* update inference errors

* Add `index_` prefix to newly-introduced error codes

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Update error messages following meilisearch/meilisearch#3301

---------

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Reject master keys shorter than 16 bytes in `production` (#209)

* Specify what happens when too short a master key is provided

* Change error message

* Characters -> bytes for master key length

* Specify Error and Warning messages regarding the master-key

* Update text/0119-instance-options.md

* Update text/0119-instance-options.md

* Specify warning messages when a master key is missing or lower than 16 bytes in env development

* Apply suggestions from code review

Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>

* Replace export MEILI_MASTER_KEY by --master-key

* Apply suggestions from code review

Co-authored-by: Louis Dureuil <louis.dureuil@gmail.com>

---------

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>
Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>

* Add `OFF` log level to spec (#211)

* Errors handling enhancement (#212)

* Replace invalid_duplicate_index_found error code to invalid_swap_duplicate_index_found

* Remove bad_request for settings properties error; replaced with dedicated error codes

* cleaning old bad usage of the spec and replaces it with relevant information

* Split search bad_request error code to dedicated error code

* Catch-up: remove unused dump_not_found error code

* Do an update pass on old specs format

* Details context, variant cases

* Add system type;Split invalid_task_date_filter to dedicated error codes

* Precise index API bad_request error code to dedicated one

* Precise bad_request to a dedicated code for the swap indexes API

* Use invalid_index_primary_key when ?primaryKey is invalid instead of bad_request; + add TODO bad_request we forgot

* marking missed bad_request on /keys

* Add missed bad_request codes for GET /keys

* Add missed bad_request codes for GET /documents

* Removes _filter suffix from query parameter error_code for tasks API; Add missing bad_request error_code on GET /tasks

* Add missing bad_request error codes on GET /indexes

* remove future possibilities

* Replace search_Parameter by search into the /search error codes naming

* Replace bad_request code when the mandatory uid is missing when posting an index resource; split the generic missing_parameter to dedicated error codes

* fix sentence sense

* Split immutable_field to dedicated api key fields

* Removes missing todo marker; Add missing part

* Add suggested review comments

* Catch-up/Add  when an invalid index uid is gen for the :indexUid path parameter

* Add immutable_index_uid error code on PATCH /indexes/:indexUid when uid is specified in the request payload

* invalid_settings_ranking_rules can only be synchronous

* Details async/sync case for error related to minWordsSizeForTypos object

* Add immutable_index_created_at and immutable_index_updated_at for the Index API Resource

* Add missing_swap_indexes on the POST swap-indexes resource

* Update text/0061-error-format-and-definitions.md

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>

* merge the :deserr_helper and the :deserr_location

* Rephrase and catch-up miss

* get rids of the index_not_accessible error code since no one remember about it

---------

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Co-authored-by: Tamo <tamo@meilisearch.com>

* Add error message when doing a migration (#213)

* Add the migration CLI error

* Update text/0105-dumps-api.md

---------

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Merge snapshot options (#214)

* Merge `--schedule-snapshot` and `--snapshot-interval-sec` options

* Update text/0119-instance-options.md

---------

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Remove any reference to the disable-auto-batching argument (#215)

Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com>

* Specify the effect of the `*` value when used in the task filters (#218)

---------

Co-authored-by: Louis Dureuil <louis.dureuil@gmail.com>
Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>
Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Co-authored-by: Tamo <tamo@meilisearch.com>
Co-authored-by: Clément Renault <clement@meilisearch.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚠️ Breaking Introduce a breaking change. Q1:2023 Ready For Review Feature specification must be reviewed. v1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants