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

Should idGenerationMode=ServerAndClientGenerated show more then throwing a 500 on 'duplicate key' #2082

Open
rduivenvoorde opened this issue Dec 19, 2024 · 4 comments

Comments

@rduivenvoorde
Copy link

rduivenvoorde commented Dec 19, 2024

Experimenting here with idGenerationMode=ServerAndClientGenerated, mostly to create a re-usable test framework (by using fixed id's).

I see that when you insert for example Thing one with id 1, and then repeat that, you only get back a 500 with: '{"code":500,"type":"error","message":"Failed to store data."}' back.

While the server logs:
[insert into "THINGS" ("NAME", "DESCRIPTION", "ID") values (?, ?, ?) returning "THINGS"."ID"]; ERROR: duplicate key value violates unique constraint "THINGS_pkey" Detail: Key ("ID")=(1) already exists.

Is there a reason to not add (al least the last part: Detail: Key ("ID")=(1) already exists to the 500 message?
Seeing: #499 this could be security thingie?

Trying to create a simple Python lib (for use in QGIS plugin) + terminal/CLI to create and search Entities here. And it is useful to present this to the user?

@rduivenvoorde
Copy link
Author

rduivenvoorde commented Dec 28, 2024

Another observations (not sure if this intended?):

  • if you generate 1000 Observations (by using 1...100 as @iot.id), so CLIENT generated id's (and FROST in ServerAndClientGenerated mode)
  • and then
  • try to POST an Observation WITHOUT @iot.id, then you get a 500 because (apparently) the sequence is not automagically updated earlier, and this POST is then trying to insert an OBSERVATION with an already used id (because sequence's last value is still 0 or 1 or so

Is this intended?

I can think of 2 scenario's:

  • the sequence should always be updated as the max(iod.id) used in the OBSERVATIONS table. In that case a new insert will never collide with an existing id?
  • the client could do this to (by doing a count first), but that does feel to not be "ServerAndClientGenerated"....

Any idea about this?

@hylkevds
Copy link
Member

hylkevds commented Jan 1, 2025

When using a client-supplied id, a check to see if this id already exists would make sense.

When mixing server-generated and client-supplied, numeric ids, one has to make sure the two use distinct ranges by manually setting the sequence to a value that doesn't conflict with the user-supplied range. Since this is very use-case specific, there is not much that FROST can do. max(id) doesn't work, since it is not transaction-safe.

I think this is mostly a documentation issue...

@rduivenvoorde
Copy link
Author

Mmm,

Yes, I'm afraid I have to agree about the mixing of id's :-)

But what about giving back a little more info then a 500 in case of conflicting id's? Would that be an issue?

@hylkevds
Copy link
Member

No, that would be a good idea. If anything, it should be a 4xx, not a 5xx, and a clearer error message should also not be too hard to implement.

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

2 participants