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

🐛 Empty query string parameters cause an internal server error #472

Closed
1 task done
nadla20xx opened this issue Jan 18, 2024 · 2 comments · Fixed by #473
Closed
1 task done

🐛 Empty query string parameters cause an internal server error #472

nadla20xx opened this issue Jan 18, 2024 · 2 comments · Fixed by #473
Assignees
Labels

Comments

@nadla20xx
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

The API has returned an error:
TypeError
Status code: 0
Message: Cannot assign string to property App\Dto\SearchCommand::$page of type Spatie\LaravelData\Optional|int
Trace: /var/www/jikan-rest/vendor/spatie/laravel-data/src/Resolvers/DataFromArrayResolver.php on line 49

Expected Behavior

The API should have returned a successful response with data.

Steps To Reproduce

Http Request: `GET /v4/anime?sfw=&unapproved=&page=&q=&sfw=&genres=&genres_exclude=&letter=&producers=&start_date=2001-01-01&end_date=2010-01-01

Environment

No response

Anything else?

No response

@pushrbx
Copy link
Collaborator

pushrbx commented Jan 19, 2024

I've triaged this. Probably we need to upgrade LaravelData package. There is a PR in LaravelData's repo which might be the solution: spatie/laravel-data#602

Otherwise some global transformer would be required, because all the endpoints are affected by this.
I mean if you set an empty param, you get an error message instead of treating it empty/non-existent.

@nadla20xx For now I can only recommend not passing empty params to the API.

@pushrbx pushrbx self-assigned this Jan 19, 2024
@pushrbx pushrbx changed the title 🐛 [OFFICIAL] Generated Issue: TypeError 🐛 Empty query string parameters cause an internal server error Jan 19, 2024
@pushrbx
Copy link
Collaborator

pushrbx commented Jan 19, 2024

Found the source of the issue, it wasn't the laravel data package version. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants