soudis commented 1 year ago

Bug report

[x] I confirm this is a bug with Supabase, not with my own application.
[x] I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

I'm using a database trigger on the storage.objects table to sum up file sizes per bucket into a different table. When the quota is exceeded the database trigger raises an exception to not allow the upload:

RAISE EXCEPTION 'Storage quota for tenant exceeded' USING MESSAGE = 'Storage quota for tenant exceeded', DETAIL='Storage quota for tenant exceeded', HINT='Upgrade your plan';

I assumed the message of the exception would be passed on by the storage api but instead i receive:

{
   message: 'Internal Server Error',
   statusCode: '500',
   error: 'internal'
}

The message from the database seems to be swallowed here for no apparent reason: https://github.com/supabase/storage-api/blob/27c3b388a45be1fafdc51706be9c60349a9f5255/src/storage/database/knex.ts#L526-L556

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

Create db trigger for objects table that raises exception
try uploading a file

Expected behavior

If the SQL exception contains details, they whould be passed on like:

{
   message: 'Storage quota exceeded',
   statusCode: '500',
   error: 'internal'
}

Could work similiar to here: https://github.com/supabase/gotrue/pull/404

System information

OS: ubuntu 22
Browser (if applies): firefox
Version of supabase-js: 2.22.0
Version of Node.js: 16.20.1

Additional context

https://github.com/supabase/gotrue/pull/404

fenos commented 1 year ago

Hi, @soudis this is somewhat international since we don't want to accidentally leak information returned from the error message from the database.

However, a good thing we can add is to return the link which points to a log line for you to inspect the error in more details, what do you think?

therealsujitk commented 5 months ago

Update

@fenos the issue in Supabase Auth was closed and they said it was planned to have similar functionality through Auth hooks where you can handle errors (reference). Maybe something similar like Storage hooks at some point in future? 🤷

this is somewhat international since we don't want to accidentally leak information returned from the error message from the database.

@fenos this concern was mentioned in supabase/auth as well, but it was decide at the time that the error message can be sent under specific conditions. ~ Refer this comment

However, this no longer works for some reason. I'm unsure if there was a reason this was removed or it it's a bug, but I've opened an issue reporting it as a bug.

I'd really like if this was possible with Supabase Auth, I don't have a requirement for this feature in Supabase Storage (at least not for now), but I mentioned this because I thought the behaviour (whatever has been decided) should be consistent.

vslio commented 5 months ago

Hi, @soudis this is somewhat international since we don't want to accidentally leak information returned from the error message from the database.

Hi @fenos, Something is definitely not right - it's leaking info for me.

This is a function that I'm using in storage RLS for selecting a file:

DECLARE
    user_subscription_exists boolean;
BEGIN
    SELECT EXISTS (
        SELECT 1 FROM subscriptions WHERE u_id = auth.uid()
    ) INTO user_subscription_exists;

    IF NOT user_subscription_exists THEN
        RAISE EXCEPTION 'NO_ACTIVE_SUBSCRIPTION' USING ERRCODE = 'P0001';
    END IF;

    RETURN user_subscription_exists;
END;

In my app, after trying to upload a file with the RLS not meeting the criteria:

const { data, error } = await supabase.storage(...)
console.log(error)

...I'm getting the following:


{"code": "DatabaseError", "error": "DatabaseError", "message": "insert into \"objects\" (\"bucket_id\", \"metadata\", \"name\", \"owner\", \"owner_id\", \"version\") values ($1, DEFAULT, $2, $3, $4, $5) - NO_ACTIVE_SUBSCRIPTION", "statusCode": "500"}```

danielpgauer commented 4 months ago

Hi, @soudis this is somewhat international since we don't want to accidentally leak information returned from the error message from the database.

Hi @fenos, Something is definitely not right - it's leaking info for me.

This is a function that I'm using in storage RLS for selecting a file:
DECLARE
    user_subscription_exists boolean;
BEGIN
    SELECT EXISTS (
        SELECT 1 FROM subscriptions WHERE u_id = auth.uid()
    ) INTO user_subscription_exists;

    IF NOT user_subscription_exists THEN
        RAISE EXCEPTION 'NO_ACTIVE_SUBSCRIPTION' USING ERRCODE = 'P0001';
    END IF;

    RETURN user_subscription_exists;
END;
In my app, after trying to upload a file with the RLS not meeting the criteria:
const { data, error } = await supabase.storage(...)
console.log(error)
...I'm getting the following:
{"code": "DatabaseError", "error": "DatabaseError", "message": "insert into \"objects\" (\"bucket_id\", \"metadata\", \"name\", \"owner\", \"owner_id\", \"version\") values ($1, DEFAULT, $2, $3, $4, $5) - NO_ACTIVE_SUBSCRIPTION", "statusCode": "500"}```

me too

supabase / storage

DB errors are not sent forward #373