[PR #1102] fix: handle errors better for duplicated urls #987

New issue

Open

opened 2026-02-25 23:36:07 +03:00 by kerem · 0 comments

kerem commented

2026-02-25 23:36:07 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/go-shiori/shiori/pull/1102
Author: @rept0id
Created: 4/30/2025
Status: 🔄 Open

Base: master ← Head: master

📝 Commits (10+)

ea80ae6 error handling / URL already exists
88f595f local-only binding for docker-compose.yml where needed
b5d7247 ...
67b5d29 URLs without prefix
928827d typo. return err for internal/core/url.go:Parse
c4440af Update internal/core/url.go
8bf20b8 Update internal/webserver/handler-api.go
34bd8e0 ...
e7858c1 ...
7b82fca ...

📊 Changes

5 files changed (+454 additions, -3 deletions)

View changed files

📝 internal/core/url.go (+53 -3)
➕ internal/core/url_test.go (+262 -0)
➕ internal/http/response/error.go (+5 -0)
📝 internal/webserver/handler-api.go (+34 -0)
➕ internal/webserver/handler-api_test.go (+100 -0)

📄 Description

This is my first pull request for this project and I'm coming ambitious. Please, show some understanding.

A. Duplicate URLs
So, I noticed that in general, adding a bookmark with a URL that already exists, results in very weird error messages.

in older versions, you get something like "json failed parsing", because the endpoint panics the API and the UI doesn't expect that
in newer versions, the UI expects such cases and it even expects error messages ! But still, all it can show is a "Unknown error occurred" because the API instead of returning a polished error it panics.

The whole case here is that for less tech savvy people, such errors make the project look like it's broken, while in reality I find it to be very stable and mature. Don't catch me wrong, but a tech savvy person my one day try to add again a URL that already added maybe some months ago and "JSON/UNKNOWN ERROR" is not something this person gets.

Let's say first, that panicking is not the best option. Specially if something is so simple as that a URL is just a duplicate. Imagine that other people may use the same deployment and they requests may fail, just because another person made a request for a duplicate URL.
For more serious errors, panicking may be optimal instead of keeping working wrongly. This is not the case here.

Changes :

Since the UI expects a polished error, I return a polished error. (http response change)
Since the UI has a kind of "modeled" way of expecting an error, I added a "ResponseHttpError" type in our model, so we can all have a common base of returning an error through our HTTP responses. (modeling change)

Thank you for your time !

B. Changed structure of docker-compose.yml.

Made bindings to host local-only where needed.
Added more restart policies.

C. URLs without "http://" or "https://" prefix.

Sometimes, someone needs to add a URL that he didn't copied pasted from the browser and wrote it by hand.
If this person doesn't add a "http://" or "https://" prefix, again that someone was getting "Unknown error occurred."
Some people doesn't even know what "https" is and other people, like me, just want to write a URL like "google.com" and not write the "https" part, like we can do in all of the modern browsers.

I made parsing of URL to retry with "https://" prefix in case of error.

P.S.
About part A :

For "ResponseHttpError" in our model, I made a file "responses" for all this in our model, there other people can add other standard responses too.

Please, I don't know how to express this in our codebase, but the responses.go in the model that I added, is not for whatever responses different endpoints return, but for some standard responses like an error response in our case.

I believe and I've seen that there are other standard responses coming in the way, so I guess this will be handful for everyone.

Maybe, just maybe, I should had named this "standardResponses.go" or just write a comment there, but to be fair I noticed that the rest of the files had one simple word as a name, so I decided to not over-engineer by myself and see the opinions of the rest of you ! Let's over-engineer all together at least. ;)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/go-shiori/shiori/pull/1102 **Author:** [@rept0id](https://github.com/rept0id) **Created:** 4/30/2025 **Status:** 🔄 Open **Base:** `master` ← **Head:** `master` --- ### 📝 Commits (10+) - [`ea80ae6`](https://github.com/go-shiori/shiori/commit/ea80ae6329ee33580ac2fc7f02f993187c1f718c) error handling / URL already exists - [`88f595f`](https://github.com/go-shiori/shiori/commit/88f595fce5087aa4b133c39994421bc1b7a44067) local-only binding for docker-compose.yml where needed - [`b5d7247`](https://github.com/go-shiori/shiori/commit/b5d724726f24da4de0c380974fb47d5314fb6762) ... - [`67b5d29`](https://github.com/go-shiori/shiori/commit/67b5d295e3f8e2f8121e68c70b0d75a3c7febca0) URLs without prefix - [`928827d`](https://github.com/go-shiori/shiori/commit/928827dc80af18911c64c8487e755febe7e30c2f) typo. return err for internal/core/url.go:Parse - [`c4440af`](https://github.com/go-shiori/shiori/commit/c4440af51459057691110f683d1b4a489292078f) Update internal/core/url.go - [`8bf20b8`](https://github.com/go-shiori/shiori/commit/8bf20b8ca5bff432702d97cec9493bc39b789839) Update internal/webserver/handler-api.go - [`34bd8e0`](https://github.com/go-shiori/shiori/commit/34bd8e0342ae1e7e2c0c5db17615ae4069e6177e) ... - [`e7858c1`](https://github.com/go-shiori/shiori/commit/e7858c1bf783d8bcb743d8bcc09ea6da1edd4ecd) ... - [`7b82fca`](https://github.com/go-shiori/shiori/commit/7b82fcad7f7cb0e0f5e9034e67f17cc729167595) ... ### 📊 Changes **5 files changed** (+454 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `internal/core/url.go` (+53 -3) ➕ `internal/core/url_test.go` (+262 -0) ➕ `internal/http/response/error.go` (+5 -0) 📝 `internal/webserver/handler-api.go` (+34 -0) ➕ `internal/webserver/handler-api_test.go` (+100 -0) </details> ### 📄 Description This is my first pull request for this project and I'm coming ambitious. Please, show some understanding. A. Duplicate URLs So, I noticed that in general, adding a bookmark with a URL that already exists, results in very weird error messages. - in older versions, you get something like "json failed parsing", because the endpoint panics the API and the UI doesn't expect that - in newer versions, the UI expects such cases and it even expects error messages ! But still, all it can show is a "Unknown error occurred" because the API instead of returning a polished error it panics. **The whole case here is that for less tech savvy people, such errors make the project look like it's broken**, while in reality I find it to be very stable and mature. Don't catch me wrong, but a tech savvy person my one day try to add again a URL that already added maybe some months ago and "JSON/UNKNOWN ERROR" is not something this person gets. **Let's say first, that panicking is not the best option. Specially if something is so simple as that a URL is just a duplicate.** Imagine that other people may use the same deployment and they requests may fail, just because another person made a request for a duplicate URL. For more serious errors, panicking may be optimal instead of keeping working wrongly. This is not the case here. Changes : - **Since the UI expects a polished error, I return a polished error.** (http response change) - **Since the UI has a kind of "modeled" way of expecting an error, I added a "ResponseHttpError" type in our model, so we can all have a common base of returning an error through our HTTP responses.** (modeling change) Thank you for your time ! B. Changed structure of docker-compose.yml. - **Made bindings to host local-only where needed.** - **Added more restart policies.** C. URLs without "http://" or "https://" prefix. Sometimes, someone needs to add a URL that he didn't copied pasted from the browser and wrote it by hand. If this person doesn't add a "http://" or "https://" prefix, again that someone was getting "Unknown error occurred." Some people doesn't even know what "https" is and other people, like me, just want to write a URL like "google.com" and not write the "https" part, like we can do in all of the modern browsers. - **I made parsing of URL to retry with "https://" prefix in case of error.** P.S. About part A : _For "ResponseHttpError" in our model, I made a file "responses" for all this in our model, there other people can add other standard responses too._ _Please, I don't know how to express this in our codebase, but the responses.go in the model that I added, is not for whatever responses different endpoints return, but for some standard responses like an error response in our case._ _I believe and I've seen that there are other standard responses coming in the way, so I guess this will be handful for everyone._ _Maybe, just maybe, I should had named this "standardResponses.go" or just write a comment there, but to be fair I noticed that the rest of the files had one simple word as a name, so I decided to not over-engineer by myself and see the opinions of the rest of you ! Let's over-engineer all together at least. ;)_ --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>