search

nudj / nudj-backend

Nudj - Backend (Archive)
0 stars 0 forks source link

Unexpected API error fetching a job #11

Closed richardbuckle closed 8 years ago

richardbuckle commented 8 years ago

The following succeeds:

curl --header "token:7EYmYZ6vanoaufMrEJiS49cmnuDYLpmlLw7GyzkuzVWSmNqCoRiPiyWfWNVI" "https://dev.nudj.co/api/v1/jobs/12"
{"data":{"id":"12","title":"Scala Dev","user":{"id":"1","name":"Robyn McGirl"}},"timestamp":1456852477.899}

whereas the following fails:

curl --header "token:7EYmYZ6vanoaufMrEJiS49cmnuDYLpmlLw7GyzkuzVWSmNqCoRiPiyWfWNVI" "https://dev.nudj.co/api/v1/jobs/14"
{"error":{"message":"Call to a member function getPrefix() on a non-object","code":10001},"timestamp":1456852485.591}

It may be relevant that the first job ID 12 is one of those returned by calling jobs/active whereas the second job ID 14 is supplied via notification because Robyn asked me for a referral.

shtukas commented 8 years ago

Is that something I should do something about ? if yes, can you give me a curl ?

richardbuckle commented 8 years ago

Assigning to Pascal to investigate please. It would be best to solve this before the Apple review begins if possible.

shtukas commented 8 years ago

note: Anything that is posted in nudj-backend is automatically assigned to me.

The test suite runs fine (just tried). Do you have a curl or at least a job number ?

shtukas commented 8 years ago

Oh... sorry, just saw it. Give me two minutes.

shtukas commented 8 years ago

( I don't get updated issues by email.... )

shtukas commented 8 years ago

It's fixed. It wasn't a problem with the code, but with the database. Job 14's database record was pointing at a user which didn't exist. I set it to Bob Tester.

The error Call to a member function getPrefix() on a non-object, was PHP's falling on its feet.

Please double check and when you are happy feel free to close the issue. Cheers,

richardbuckle commented 8 years ago

It's working now, thanks.

Closing this issue, I see two areas that may need more thought in due course. For now let's just put them on the thinking list.

  1. It was possible to get the database into a state where referential integrity was broken (or was this a manual edit?)
  2. A more informative error message, such as user <n> not found for job 14, would be better in cases where referential integrity was broken (or any other preconditions are not met). But that might be too much work.
shtukas commented 8 years ago

(1) It was the manual edit that I made when you reported that the phone numbers you had given to me for Alice and Bob where breaking the code (the code was reporting them as incorrect numbers if you remember). I then replied to you that I could not explain the error but that I rebuilt the database records. Unfortunately the auto increment primary key (I hate those things) broke the foreign reference.

(2) Most of that code (and notably the code that threw the error) is third party ORM code. I will do something about it (after the code freeze during Apple's review), but then doing it properly will take more effort that just rewriting the entire backend in a real language.

Cheers,

richardbuckle commented 8 years ago

In such cases then it becomes a business decision about managing our technical debt, so we should definitely step back and evaluate matters, with input from @RobynMcG, before expending any effort to shore up an implementation that we may well decide to discard.

shtukas commented 8 years ago

Well.... I should have shut up, as usual... :)

That being said, after the work I did of actually splitting nujd-api, nujd-web-application and nudj-desk in independent entities (a decision controversial at the time), it not only makes sense, but I would actually welcome rewriting nujd-api, as a low priority background work, in a more robust language.