fix: handle queries on non-existing table gracefully

Add new `TableNotFound` error and add json error
message for this error. Closes #3697.

Author: taimoorzaeem

CHANGELOG.md

@@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez
  - #3779, Always log the schema cache load time - @steve-chavez
  - #3706, Fix insert with `missing=default` uses default value of domain instead of column - @taimoorzaeem
+ - #3697, Handle queries on non-existing table gracefully - @taimoorzaeem
 
 ### Changed
 

docs/references/errors.rst

@@ -294,6 +294,10 @@ Related to a :ref:`schema_cache`. Most of the time, these errors are solved by :
 |               |             | in the ``columns`` query parameter is not found.            |
 | PGRST204      |             |                                                             |
 +---------------+-------------+-------------------------------------------------------------+
+| .. _pgrst205: | 404         | Caused when the :ref:`table specified <tables_views>` in    |
+|               |             | the URI is not found.                                       |
+| PGRST205      |             |                                                             |
++---------------+-------------+-------------------------------------------------------------+
 
 .. _pgrst3**:
 

src/PostgREST/ApiRequest/Types.hs

@@ -91,6 +91,7 @@ data ApiRequestError
   | UnacceptableSchema [Text]
   | UnsupportedMethod ByteString
   | ColumnNotFound Text Text
+  | TableNotFound Text
   | GucHeadersError
   | GucStatusError
   | OffLimitsChangesError Int64 Integer

src/PostgREST/Error.hs

@@ -86,6 +86,7 @@ instance PgrstError ApiRequestError where
   status UnsupportedMethod{}         = HTTP.status405
   status LimitNoOrderError           = HTTP.status400
   status ColumnNotFound{}            = HTTP.status400
+  status TableNotFound{}             = HTTP.status404
   status GucHeadersError             = HTTP.status500
   status GucStatusError              = HTTP.status500
   status OffLimitsChangesError{}     = HTTP.status400
@@ -253,6 +254,9 @@ instance JSON.ToJSON ApiRequestError where
   toJSON (ColumnNotFound relName colName) = toJsonPgrstError
     SchemaCacheErrorCode04 ("Could not find the '" <> colName <> "' column of '" <> relName <> "' in the schema cache") Nothing Nothing
 
+  toJSON (TableNotFound relName) = toJsonPgrstError
+    SchemaCacheErrorCode05 ("Could not find relation '" <> relName <> "' in the schema cache") Nothing Nothing
+
 -- |
 -- If no relationship is found then:
 --
@@ -640,6 +644,7 @@ data ErrorCode
   | SchemaCacheErrorCode02
   | SchemaCacheErrorCode03
   | SchemaCacheErrorCode04
+  | SchemaCacheErrorCode05
   -- JWT authentication errors
   | JWTErrorCode00
   | JWTErrorCode01
@@ -689,6 +694,7 @@ buildErrorCode code = case code of
   SchemaCacheErrorCode02 -> "PGRST202"
   SchemaCacheErrorCode03 -> "PGRST203"
   SchemaCacheErrorCode04 -> "PGRST204"
+  SchemaCacheErrorCode05 -> "PGRST205"
 
   JWTErrorCode00         -> "PGRST300"
   JWTErrorCode01         -> "PGRST301"

src/PostgREST/Plan.hs

@@ -52,7 +52,7 @@ import PostgREST.SchemaCache                 (SchemaCache (..))
 import PostgREST.SchemaCache.Identifiers     (FieldName,
                                               QualifiedIdentifier (..),
                                               RelIdentifier (..),
-                                              Schema)
+                                              Schema, dumpQi)
 import PostgREST.SchemaCache.Relationship    (Cardinality (..),
                                               Junction (..),
                                               Relationship (..),
@@ -152,14 +152,14 @@ dbActionPlan dbAct conf apiReq sCache = case dbAct of
 
 wrappedReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Bool -> Either Error CrudPlan
 wrappedReadPlan  identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{..},..} headersOnly = do
-  rPlan <- readPlan identifier conf sCache apiRequest
+  rPlan <- readPlan False identifier conf sCache apiRequest
   (handler, mediaType)  <- mapLeft ApiRequestError $ negotiateContent conf apiRequest identifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
   if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
   return $ WrappedReadPlan rPlan SQL.Read handler mediaType headersOnly identifier
 
 mutateReadPlan :: Mutation -> ApiRequest -> QualifiedIdentifier -> AppConfig -> SchemaCache -> Either Error CrudPlan
 mutateReadPlan  mutation apiRequest@ApiRequest{iPreferences=Preferences{..},..} identifier conf sCache = do
-  rPlan <- readPlan identifier conf sCache apiRequest
+  rPlan <- readPlan False identifier conf sCache apiRequest
   mPlan <- mutatePlan mutation identifier apiRequest sCache rPlan
   if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
   (handler, mediaType)  <- mapLeft ApiRequestError $ negotiateContent conf apiRequest identifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
@@ -173,7 +173,7 @@ callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferenc
   proc@Function{..} <- mapLeft ApiRequestError $
     findProc identifier paramKeys (dbRoutines sCache) iContentMediaType (invMethod == Inv)
   let relIdentifier = QualifiedIdentifier pdSchema (fromMaybe pdName $ Routine.funcTableName proc) -- done so a set returning function can embed other relations
-  rPlan <- readPlan relIdentifier conf sCache apiRequest
+  rPlan <- readPlan True relIdentifier conf sCache apiRequest
   let args = case (invMethod, iContentMediaType) of
         (InvRead _, _)      -> DirectArgs $ toRpcParams proc qsParams'
         (Inv, MTUrlEncoded) -> DirectArgs $ maybe mempty (toRpcParams proc . payArray) iPayload
@@ -320,8 +320,8 @@ resolveQueryInputField ctx field = withTextParse ctx $ resolveTypeOrUnknown ctx
 -- | Builds the ReadPlan tree on a number of stages.
 -- | Adds filters, order, limits on its respective nodes.
 -- | Adds joins conditions obtained from resource embedding.
-readPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Either Error ReadPlanTree
-readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregates} SchemaCache{dbTables, dbRelationships, dbRepresentations} apiRequest  =
+readPlan :: Bool -> QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Either Error ReadPlanTree
+readPlan fromCallPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregates} SchemaCache{dbTables, dbRelationships, dbRepresentations} apiRequest  =
   let
     -- JSON output format hardcoded for now. In the future we might want to support other output mappings such as CSV.
     ctx = ResolverContext dbTables dbRepresentations qi "json"
@@ -340,7 +340,8 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregate
     addLogicTrees ctx apiRequest =<<
     addRanges apiRequest =<<
     addOrders ctx apiRequest =<<
-    addFilters ctx apiRequest (initReadRequest ctx $ QueryParams.qsSelect $ iQueryParams apiRequest)
+    addFilters ctx apiRequest =<<
+    searchTable fromCallPlan qi dbTables (initReadRequest ctx $ QueryParams.qsSelect $ iQueryParams apiRequest)
 
 -- Build the initial read plan tree
 initReadRequest :: ResolverContext -> [Tree SelectItem] -> ReadPlanTree
@@ -738,6 +739,14 @@ validateAggFunctions aggFunctionsAllowed (Node rp@ReadPlan {select} forest)
   | not aggFunctionsAllowed && any (isJust . csAggFunction) select = Left AggregatesNotAllowed
   | otherwise = Node rp <$> traverse (validateAggFunctions aggFunctionsAllowed) forest
 
+-- We only search for the table when readPlan is not called from call plan. This
+-- is because we reuse readPlan in callReadPlan to search for function name
+searchTable :: Bool -> QualifiedIdentifier -> TablesMap -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
+searchTable fromCallPlan qi tbls readPlanTree =
+  case (fromCallPlan, HM.lookup qi tbls) of
+    (False, Nothing) -> Left (TableNotFound $ dumpQi qi)
+    _                -> Right readPlanTree
+
 addFilters :: ResolverContext -> ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
 addFilters ctx ApiRequest{..} rReq =
   foldr addFilterToNode (Right rReq) flts
@@ -961,7 +970,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{
     typedColumnsOrError = resolveOrError ctx tbl `traverse` S.toList iColumns
 
 resolveOrError :: ResolverContext -> Maybe Table -> FieldName -> Either ApiRequestError CoercibleField
-resolveOrError _ Nothing _ = Left NotFound
+resolveOrError ResolverContext{qi} Nothing _ = Left $ TableNotFound $ dumpQi qi
 resolveOrError ctx (Just table) field =
   case resolveTableFieldName table field of
     CoercibleField{cfIRType=""} -> Left $ ColumnNotFound (tableName table) field

test/io/test_big_schema.py

@@ -65,6 +65,6 @@ def test_should_not_fail_with_stack_overflow(defaultenv):
 
     with run(env=env, wait_max_seconds=30) as postgrest:
         response = postgrest.session.get("/unknown-table?select=unknown-rel(*)")
-        assert response.status_code == 400
+        assert response.status_code == 404
         data = response.json()
-        assert data["code"] == "PGRST200"
+        assert data["code"] == "PGRST205"

test/io/test_io.py

@@ -973,11 +973,10 @@ def test_log_level(level, defaultenv):
                 r'- - postgrest_test_anonymous \[.+\] "GET /unknown HTTP/1.1" 404 - "" "python-requests/.+"',
                 output[2],
             )
+
+            assert len(output) == 5
             assert "Connection" and "is available" in output[3]
-            assert "Connection" and "is available" in output[4]
-            assert "Connection" and "is used" in output[5]
-            assert "Connection" and "is used" in output[6]
-            assert len(output) == 7
+            assert "Connection" and "is used" in output[4]
 
 
 def test_no_pool_connection_required_on_bad_http_logic(defaultenv):

test/spec/Feature/ConcurrentSpec.hs

@@ -27,8 +27,8 @@ spec =
           `shouldRespondWith` [json|
               { "hint": null,
                 "details":null,
-                "code":"42P01",
-                "message":"relation \"test.fakefake\" does not exist"
+                "code":"PGRST205",
+                "message":"Could not find relation 'test.fakefake' in the schema cache"
               } |]
           { matchStatus  = 404
           , matchHeaders = []

test/spec/Feature/Query/DeleteSpec.hs

@@ -109,7 +109,15 @@ spec =
 
     context "totally unknown route" $
       it "fails with 404" $
-        request methodDelete "/foozle?id=eq.101" [] "" `shouldRespondWith` 404
+        request methodDelete "/foozle?id=eq.101" [] ""
+          `shouldRespondWith` [json|
+            { "hint": null, "details":null,
+              "code":"PGRST205",
+              "message":"Could not find relation 'test.foozle' in the schema cache"
+              } |]
+          { matchStatus = 404
+          , matchHeaders = []
+          }
 
     context "table with limited privileges" $ do
       it "fails deleting the row when return=representation and selecting all the columns" $

test/spec/Feature/Query/InsertSpec.hs

@@ -477,7 +477,7 @@ spec actualPgVersion = do
             {"id": 204, "body": "yyy"},
             {"id": 205, "body": "zzz"}]|]
           `shouldRespondWith`
-          [json|{} |]
+          [json|{"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.garlic' in the schema cache"} |]
           { matchStatus  = 404
           , matchHeaders = []
           }