Promise handling is "too optimistic" (url_path not existing in old entities)
jsanta opened this issue · 9 comments
Bug report
Describe the bug
Existing entities created before installing the url-alias plugin do not have the url-path column. After installing the plugin many api requests fail because of the preparePath service Promise.all not handling any errors.
Checking out the code, most promises are used in the async-await style, skipping any error handling, which is a "too optimistic way" of managing Promises.
Steps to reproduce the behavior
- Install Strapi without plugins
- Create and populate a couple of entities
- Stop Strapi nd install url-alias plugin
- See error:
error: Cannot
read properties of null (reading 'url_path')
TypeError: Cannot read properties of nul
l (reading 'url_path')
Expected behavior
No server errors when using the api directly (not using any url-alias).
System
- Node.js version: 16.15.1
- NPM version: 8.11.0
- Strapi version: 4.3.3
- Plugin version: 1.0.0-alpha.4
- Database: Postgres
- Operating system: Linux, Mac
Workaround
In file: node_modules/@strapi-community/strapi-plugin-url-alias/server/content-api/services/prepare-path.js
At the end of the Promise.al on line 18 (should be line 32), add a catch:
.catch(_err => { return; })
Hi @jsanta
Thanks for reporting the issue.
I'll take a look at this probably later this week :)
Hi @jsanta,
I believe this issue comes forward because url_path_id is null in this case of pre-existing records.
I've made the prepare-path service more strict to only rewrite the result if url_path_id is not null. See PR #12.
Could you test this out locally? You can install the PR like this:
yarn add strapi-community/strapi-plugin-url-alias#pull/12/head
npm install strapi-community/strapi-plugin-url-alias#pull/12/head
This was released with version 1.0.0-alpha.5 of the plugin.
If you're still experiencing the issue beyond this version feel free to re-open the GH issue or create a new one.
Haven´t tried it yet. Had to patch some things on the plugin to match certain specific requirements (I was hoping that URL alias returned the whole entity and not only the higher level attributes, in order to act as an alias and skip building the whole query, otherwise having the plugin would not mean added value as you could simply add an url_path column to all aliased entities).
Why do you want to have the whole url_path entity? You’re just making use of the path field, not the other fields on the entity.
Storing the URLs in a seperate table makes it easy for you to query a record based on the URL regardless of the type. As you just query the url_path table and see what entity is related to the specific URL.
Also having them in a seperate table helps with requiring uniqueness of URLs.
So there are def some benefits as aposed to just having a url_path column on the different tables.
For eg a blog entry. /blog/fancy-title-here as the url alias
As a Strapi user (non dev) I would like to have as a response the whole entity, by simply using the url-alias as a parameter. No need to know the whole entity structure, neither how to build a populated Strapi query.
Building queries should be a developer's task.
But that's a particular use case, I guess I misunderstood the plugin's main goal.
I’m open to feature requests.
Though I’m not sure I fully understand. As there is a way to query the whole entity by using just the URL alias as a parameter.
Like this:
http://localhost:1337/api/url-alias/get?path=/blog/fancy-title-here
EDIT:
Or did you expect to have all the relations populated by default here? Without adding the population params?
If that is what you want I could add a boolean setting to enable this.
Just that. Without population params. I hot patched url-alias for that, using the strapi-deep code with some micro-optimizations for the recursion.
Had to add a exclude para though, because we have an entity that causes an infinite population.
Will try to make a pull request with these changes.
Sounds good! Looking forward to it.