PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.38k stars 1.03k forks source link

Stack Overflow error in dev enviroment when table and embed resource do not exist #3329

Closed laurenceisla closed 6 months ago

laurenceisla commented 7 months ago

Environment

Description of issue

When requesting a table that does not exist with any embedded resource, it shows a stack overflow error in the logs and fails the request. To reproduce:

$ nix-shell --pure --run "postgrest-with-postgresql-16 -f ./test/spec/fixtures/load.sql postgrest-run"
$ curl "localhost:3000/x?select=y(*)"
curl: (52) Empty reply from server

Expected:

$ curl "localhost:3000/x?select=y(*)"
{
  "code":"PGRST200",
  "details":"Searched for a foreign key relationship between 'x' and 'y' in the schema 'public', but no matches were found.",
  "hint":null,
  "message":"Could not find a relationship between 'x' and 'y' in the schema cache"
}

This only happens in the Nix development environment for now (not for spec tests), since it enables the leak detection with -K1K:

https://github.com/PostgREST/postgrest/blob/ec7ab271d69e8ad907328b34c7d1a4aed1c60af9/postgrest.cabal#L185

It appears that the fuzzyset package, used to build the "hint" of the error, is the culprit. The evidence is that it is marked as "broken" in the latest update of nixpkgs/GHC in commit 6682baeb (previous commits do not throw the error even with -K1K):

https://github.com/PostgREST/postgrest/blob/ec7ab271d69e8ad907328b34c7d1a4aed1c60af9/nix/overlays/haskell-packages.nix#L48-L49

steve-chavez commented 7 months ago

Hm, so the solution should be pinning a previous version of fuzzyset?

laurenceisla commented 7 months ago

Hm, so the solution should be pinning a previous version of fuzzyset?

At first glance, it should. But IIRC, previous versions have conflicts with the current GHC version. Particularly the base and text dependencies, I think (I may be wrong).

steve-chavez commented 7 months ago

Seems related https://github.com/commercialhaskell/stackage/issues/7341#issuecomment-1984556655

wolfgangwalther commented 7 months ago

The evidence is that it is marked as "broken" in the latest update of nixpkgs/GHC in commit https://github.com/PostgREST/postgrest/commit/6682baebd21253ebb851d8c58548982736bc0714 (previous commits do not throw the error even with -K1K):

I don't think this is related. broken just means it failed to build in nixpkgs at some point in the past. Nowadays it does build fine - thus I marked it unbroken in that PR.

But IIRC, previous versions have conflicts with the current GHC version.

Correct, we can't downgrade. But I don't see why we should here, I don't see a connection to fuzzyset here, yet.

wolfgangwalther commented 7 months ago

latest update of nixpkgs/GHC in commit https://github.com/PostgREST/postgrest/commit/6682baebd21253ebb851d8c58548982736bc0714 (previous commits do not throw the error even with -K1K):

Of course the only code changes in that are about fuzzyset, so that might very well be an indication that this is related. It's just the broken part that is certainly unrelated here. There is fuzzyset 0.3.2 available on hackage now, so maybe we first try updating the fuzzyset package to see whether that makes a difference.

wolfgangwalther commented 7 months ago

The most likely explanation though is, that I just broke it with the code changes about fuzzyset that I made. So it might be worth reviewing them carefully.

laurenceisla commented 7 months ago

It's just the broken part that is certainly unrelated here.

Ah, I thought of it as evidence that something happened there. But cool, it isn't related to a broken library, then.

There is fuzzyset 0.3.2 available on hackage now, so maybe we first try updating the fuzzyset package

The most likely explanation though is, that I just broke it with the code changes about fuzzyset that I made.

Cool, will do both to check what's happening.

laurenceisla commented 7 months ago

Cool, will do both to check what's happening.

Tried to upgrade to 0.3.2, but the problem persisted. Then, after checking the library's code, I used the most similar function to Fuzzy.getOne, which is Fuzzy.closestMatch, but the error was still there. I also mixed some code between the old/new library versions to no avail.

It may be that the library itself has the leaks, not really sure. Regardless, I think the best course of action is to downgrade to 0.2.4, which is better than the version we had before because it's compatible with the other packages we have pinned in postgrest.cabal.