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

getOrCreate, Error Handler and CreateIndex Usage changes #436

Merged
merged 20 commits into from
Jun 16, 2020

Conversation

bidoubiwa
Copy link
Contributor

@bidoubiwa bidoubiwa commented Jun 8, 2020

In this PR

CreateIndex Usage changes

Previously

client.createIndex(data: IndexRequest): Promise<Index>

New usage:

client.createIndex(uid: string, options?: IndexOptions): Promise<Index>

Example

await client.createIndex('movies', {
  primaryKey: "movieId"
})

Error Handler

  • Http error handler to dispatch to the right error
function httpErrorHandler(e: AxiosError, cachedStack?: string): void {
  if (e.response !== undefined) {
    throw new MeiliSearchApiError(e, cachedStack)
  } else if (e.isAxiosError) {
    throw new MeilISearchCommunicationError(e.message)
  } else {
    throw e
  }
}
  • Create additional error meiliSearchCommunicationError when a problem occurred when trying to communicate with MeiliSearch.

Refactor

  • Changed the name of the folder custom-errors to errors
  • Changed meilisearchError to meilisearchApiError

NEW: AddOrCreateIndex

  • Updated example with getOrCreateIndex
  • Added getOrCreateIndex in the README references
  • Tests for getOrCreateIndex

If movies does not exist it will be created in MeiliSearch and will return an index instance.
If it already exists, it returns an index instance.

const index = await client.addOrCreateIndex('movies', {
  primaryKey: "movieId"
})

@bidoubiwa bidoubiwa marked this pull request as ready for review June 9, 2020 08:14
@curquiza
Copy link
Member

curquiza commented Jun 9, 2020

Http error handler to dispatch to the right error

What is the default error?
According to this issue, the default one should be MeiliSearchError.

Add Or Create Index

I wait for the Samuel answer in this issue before reviewing.

@bidoubiwa
Copy link
Contributor Author

MeiliSearchError: for any other errors in the SDK (wrong parameter if the SDK checks the parameters...etc)

Since we do not check anything in the SDK for the moment I did not add this error. For any native error as:
EvalError, InternalError, RangeError, ReferenceError, SyntaxError, TypeError, URIError
they are being passed down with throw e.

@curquiza
Copy link
Member

curquiza commented Jun 9, 2020

Ok, but what is e exactly? What does it contain? 🙂

@bidoubiwa bidoubiwa changed the title Get or create index Get or create index and error handler Jun 9, 2020
@bidoubiwa
Copy link
Contributor Author

bidoubiwa commented Jun 9, 2020

e is the Error object.
By default it is the global error object:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error
But in the case it was returned by Axios, it is an axiosError that inherits the default Error object.

@curquiza
Copy link
Member

curquiza commented Jun 11, 2020

@bidoubiwa, according to this comment:

  • You can now update the getOrCreateIndex method in this PR
  • You can create a new PR that modifies createIndex
    🙂

@bidoubiwa bidoubiwa changed the title Get or create index and error handler getOrCreate, Error Handler and CreateIndex Usage changes Jun 11, 2020
README.md Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
@bidoubiwa bidoubiwa requested a review from curquiza June 16, 2020 09:17
CHANGELOG.md Outdated Show resolved Hide resolved
bidoubiwa and others added 2 commits June 16, 2020 11:35
Co-authored-by: Clémentine Urquizar <clementine@meilisearch.com>
@bidoubiwa bidoubiwa merged commit 4bdfad4 into bump-meilisearch-v0.11 Jun 16, 2020
@bidoubiwa bidoubiwa deleted the get_or_create_index branch June 16, 2020 11:40
@curquiza curquiza mentioned this pull request Jun 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants