Skip to content

Getting empty 'errors' result #48

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

Closed
GregPeden opened this issue Feb 27, 2017 · 10 comments
Closed

Getting empty 'errors' result #48

GregPeden opened this issue Feb 27, 2017 · 10 comments

Comments

@GregPeden
Copy link
Contributor

I have a case where the API is returning 'errors' with an empty array. Since it's handled by the API and not the exception handler, error tracing this is very difficult. I suspect it is an error event for which no message is provided in the default config.

I am not familiar enough with this package to trace the error generation at this time, otherwise I might try to provide a better hint!

So, two suggestions:

  1. If there are error events without a message, maybe they should be added to the json-api-errors.php?

  2. Perhaps a default error message should be provided, and when this default is used a log dump to the server log file should occur?

@lindyhopchris
Copy link
Member

Hmm, yes, agree. What is the HTTP status code when you get these errors? (That will help me track down where it's coming from).

@zyuzka
Copy link

zyuzka commented Feb 28, 2017

When we send an id for a nonexistent record, we receive an empty errors[] and 404 status.
The same was found with 500 error - when we send id = 0.

@lindyhopchris
Copy link
Member

Ok, thanks for the pointers.

The one proviso I'd say about all of this is that there is no requirement in the spec to send an error object - i.e. an empty errors array is valid.

E.g. in the 404 setting I'd suggest that the HTTP status itself tells the client what the problem is. Same with 500 - i.e. tells the client that an internal server error has occurred and it's unlikely you'd want to give any specific detail about what has happened anyway. (Of course your application should have logged the problem itself.)

However, as far as I can remember there is a way to customise this in your error config though so I'll check on that and get back to you.

@GregPeden
Copy link
Contributor Author

I do get a "404 Not found" response code. In my case, I am getting this on a relationship link. So, the base object exists but then when I follow the relationship link I get "404 not found". Probably there is something with my work that is causing it, I just don't know where the failure occurs. Relationship links on other models do work fine.

One thing which comes to mind, my 'id' on the country object below is alphabetical, using the UN codes for countries (ex. Canada = "CA", United States = "US"). If that is a problem I'll switch to numeric IDs however it works fine with Laravel Eloquent. The direct URL to a country with alphabetic 'id' works fine.

Also, I am using 'country-divisions' for the name of provinces/states/etc. in the JSON API but the relationships in Eloquent are called 'division()'. I think I have that set up right but not sure, however it shouldn't apply in this specific case I don't think since I'm calling the 'country()' relationship.

Base URL (works):
http://hoistway.dev/api/v1/country-divisions/1

Relationship URL (get 404 error):
http://hoistway.dev/api/v1/country-divisions/1/country

Direct link works
http://hoistway.dev/api/v1/countries/CA

@lindyhopchris
Copy link
Member

lindyhopchris commented Mar 1, 2017

@SirLamer non-numeric keys should work fine, we're using UUIDs for some of our resources in one of our apps.

The relationship that isn't working, have you added it to the properties on your Request class? There's a hasOne and hasMany property that needs the relationship name in them.

E.g.:
https://github.com/cloudcreativity/demo-laravel-json-api/blob/5.4/app/JsonApi/Posts/Request.php#L13-L23

@GregPeden
Copy link
Contributor Author

Okay that is it, but this is a "belongsTo" relationship so if all relationships should go in there then it should be renamed. Same with "hasMany" and "belongsToMany". Just call it "singularRelationships" and "pluralRelationships" or something like that.

@lindyhopchris
Copy link
Member

They are hasOne and hasMany from the JSON API perspective. JSON API doesn't have belongsToMany etc. Which is why they're called that.

@GregPeden
Copy link
Contributor Author

OOOHHHHHHHHHH okay then!

I wonder if maybe the comment on the hasOne and hasMany variables should mention that in the docblock? Same with the relationship methods used in the validation class. I probably won't be the last to confuse that. ;-)

My initial understanding took the assumed perspective of something like "I guess they need to know these ones because the ID reference to this model is stored in other models, so they need to know what to look up."

@lindyhopchris
Copy link
Member

@SirLamer yes you're right, it's not actually very clear at the moment.

For info we're planning on moving the definition of which relationships should be exposed as endpoints to the route registration. The detail of what is going to change is here:
#53

@lindyhopchris
Copy link
Member

Closing this as the routing refactor allows control of what routes are registered - currently on develop and will be released a v0.8

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

No branches or pull requests

3 participants