directus / v8-archive

Directus Database API — Wraps Custom SQL Databases with a REST/GraphQL API
https://docs.directus.io/api/reference.html
505 stars 204 forks source link

Error when updating WYSIWYG interface after upgrade to v8.5.0 #1722

Closed stevecoleman100 closed 4 years ago

stevecoleman100 commented 4 years ago

I just updated via GIT from v8.4.0 to v8.5.0 On the app I go to Settings > Collections & Fields and select a current collection. I click on an existing WYSIWYG interface and add fullscreen from the options. On the front end I get an error:

Trouble processing request. Try again after refreshing the page

On the console I get this error: Error: "Statement could not be executed (42000 - 1064 - You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '(16777215) Null' at line 1)" a API.js:39 request API.js:253 error.js:6:22 Ud error.js:6 VueJS 2 e fields.vue:448 l runtime.js:45 O runtime.js:271 a runtime.js:97 n asyncToGenerator.js:3 m asyncToGenerator.js:29

Changing MySQL Datatype from MEDIUMTEXT to TEXT at the same time as adding fullscreen option allows the updated setting to be saved.

When I click back into the interface settings the MySQL Datatype reverts back to MEDIUMTEXT, but the fullscreen option is preserved offering a workaround.

benhaynes commented 4 years ago

@dur41d — any thoughts on this?

https://github.com/directus/app/pull/2480

rijkvanzanten commented 4 years ago

I can't reproduce this on my end nor on the demo. @blob100 what version of MariaDB are you using?

nachogarcia commented 4 years ago

I finally updated (I'm using docker) from 8.3 to 8.5.1 and had no issues. (mysql 5.7)

dur41d commented 4 years ago

I was able to reproduce it in my ddev environement. The error happens in Directus\Api\Routes\Fields->update. For some reason it tries to update the table schema which i don't think is correct. This statement is failing:

ALTER TABLE `news` MODIFY COLUMN `ewwe` LONGTEXT(4294967295) Null

Any idea why it's trying the change the table schema in the first place?

Server: db via TCP/IP Server type: MariaDB Server version: 10.2.27-MariaDB-1:10.2.27+maria~bionic-log - mariadb.org binary distribution Protocol version: 10 User: db@172.19.0.4 Server charset: UTF-8 Unicode (utf8)

dur41d commented 4 years ago

BTW, I saw this statement in Directus\Api\Routes\Fields and it don't think it's a practice to catch and throw and SQLException. It does nothing but losing the stack trace of the original exception. I think the try block should be removed.

 try {
            $responseData = $service->changeColumn(
                $request->getAttribute('collection'),
                $request->getAttribute('field'),
                $request->getParsedBody(),
                $request->getQueryParams()
            );
        } catch (\Exception $e) {
            throw new SQLException($e->getMessage());
        }
dur41d commented 4 years ago

The statement would work the length is removed like this:

ALTER TABLE `news` MODIFY COLUMN `ewwe` LONGTEXT Null

I think the problem is with putting the length in the LONGTEXT type field. I think the length should be omitted from this type of field.

WoLfulus commented 4 years ago

@dur41d are you able to get a stacktrace? it might help tracing the issue

thanks :)

dur41d commented 4 years ago

I created a PR for this.

https://github.com/directus/api/pull/1727

stevecoleman100 commented 4 years ago

Sorry for the delay - timezones. The original error was when using MySQL Version: 10.1.40-MariaDB-cll-lve

benhaynes commented 4 years ago

Not sure if this is the cause, but Directus says in the docs that MariaDB 10.2+ is required.

dur41d commented 4 years ago

I updated to the MariaDB 10.4 and the error still happens.

dur41d commented 4 years ago

According to this only TEXT allows optional length variable. TINYTEXT, MEDIUMTEXT, LONGTEXT don't allow length variable. I'll update the PR to set length=null for those data types.

dur41d commented 4 years ago

I was able to reproduce this with MySQL v8.0.17. The reason is that directus is producing an invalid alter statement for longtext and mediumtext datatypes. This is not new bug and I was able to reproduce it with v8.4. I think it's just that nobody tried to update a wysiwyg field before adding the fullscreen option. The erroneous statement looks like this:

ALTER TABLE `news` MODIFY COLUMN `[column name]` LONGTEXT(4294967295) Null

This is the correct statement without the length:

ALTER TABLE `news` MODIFY COLUMN `[column name]` LONGTEXT Null

steps to reproduce:

  1. Create a collection with wysiwyg field
  2. Try to update the field by unchecking one of the toolbar options.

I'm not sure why directus is issuing an alter statement to begin with. I traced the problem to this function in TableService.php that is evaluating true but it does not make sense to me as the column schema does not need an update when changing the toolbar options. But i'm still new to the code base and this is beyond me to fix for now.

protected function shouldUpdateSchema(array $data)
    {
        // NOTE: If any of these attributes exists the database needs to update the column
        return ArrayUtils::containsSome($data, [
            'type',
            'datatype',
            'unique',
            'primary_key',
            'auto_increment',
            'length',
            'note',
            'signed',
            'nullable',
        ]);
    }
everyx commented 4 years ago

This problem also appear when modify repeater interface options with MySQL v5.7.28

api_1             | [2020-02-08 12:27:18] api[xxxxxx].ERROR: Directus\Exception\SQLException: Statement could not be executed (42000 - 1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '(16777215) Null' at line 1) in /var/www/html/src/endpoints/Fields.php:122
api_1             | Stack trace:
api_1             | #0 [internal function]: Directus\Api\Routes\Fields->update
api_1             | #1 /var/www/html/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php(40): call_user_func
api_1             | #2 /var/www/html/vendor/slim/slim/Slim/Route.php(281): Slim\Handlers\Strategies\RequestResponse->__invoke
api_1             | #3 /var/www/html/src/core/Directus/Application/Http/Middleware/AbstractRateLimitMiddleware.php(34): Slim\Route->__invoke
api_1             | #4 [internal function]: Directus\Application\Http\Middleware\AbstractRateLimitMiddleware->__invoke
api_1             | #5 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #6 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #7 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #8 /var/www/html/src/core/Directus/Application/Http/Middleware/AuthenticationMiddleware.php(124): Slim\Route->Slim\{closure}
api_1             | #9 [internal function]: Directus\Application\Http\Middleware\AuthenticationMiddleware->__invoke
api_1             | #10 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #11 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #12 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #13 /var/www/html/src/core/Directus/Application/Http/Middleware/TableGatewayMiddleware.php(25): Slim\Route->Slim\{closure}
api_1             | #14 [internal function]: Directus\Application\Http\Middleware\TableGatewayMiddleware->__invoke
api_1             | #15 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #16 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #17 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #18 /var/www/html/src/core/Directus/Application/Http/Middleware/DatabaseMigrationMiddleware.php(15): Slim\Route->Slim\{closure}
api_1             | #19 [internal function]: Directus\Application\Http\Middleware\DatabaseMigrationMiddleware->__invoke
api_1             | #20 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #21 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #22 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #23 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\Route->Slim\{closure}
api_1             | #24 /var/www/html/vendor/slim/slim/Slim/Route.php(268): Slim\Route->callMiddlewareStack
api_1             | #25 /var/www/html/vendor/slim/slim/Slim/App.php(503): Slim\Route->run
api_1             | #26 /var/www/html/src/core/Directus/Application/Http/Middleware/AbstractRateLimitMiddleware.php(34): Slim\App->__invoke
api_1             | #27 [internal function]: Directus\Application\Http\Middleware\AbstractRateLimitMiddleware->__invoke
api_1             | #28 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #29 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #30 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #31 /var/www/html/vendor/directus/proxy-detection/src/ProxyDetectionMiddleware.php(30): Slim\App->Slim\{closure}
api_1             | #32 /var/www/html/src/core/Directus/Application/Http/Middleware/ProxyMiddleware.php(18): RKA\Middleware\ProxyDetectionMiddleware->__invoke
api_1             | #33 [internal function]: Directus\Application\Http\Middleware\ProxyMiddleware->__invoke
api_1             | #34 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #35 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #36 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #37 /var/www/html/vendor/akrabat/ip-address-middleware/src/IpAddress.php(113): Slim\App->Slim\{closure}
api_1             | #38 [internal function]: RKA\Middleware\IpAddress->__invoke
api_1             | #39 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #40 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #41 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #42 /var/www/html/src/core/Directus/Application/Http/Middleware/CorsMiddleware.php(71): Slim\App->Slim\{closure}
api_1             | #43 [internal function]: Directus\Application\Http\Middleware\CorsMiddleware->__invoke
api_1             | #44 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #45 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #46 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #47 /var/www/html/src/core/Directus/Application/Http/Middleware/ResponseCacheMiddleware.php(62): Slim\App->Slim\{closure}
api_1             | #48 [internal function]: Directus\Application\Http\Middleware\ResponseCacheMiddleware->__invoke
api_1             | #49 /var/www/html/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array
api_1             | #50 [internal function]: Slim\DeferredCallable->__invoke
api_1             | #51 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func
api_1             | #52 /var/www/html/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\App->Slim\{closure}
api_1             | #53 /var/www/html/vendor/slim/slim/Slim/App.php(392): Slim\App->callMiddlewareStack
api_1             | #54 /var/www/html/vendor/slim/slim/Slim/App.php(297): Slim\App->process
api_1             | #55 /var/www/html/src/core/Directus/Application/Application.php(161): Slim\App->run
api_1             | #56 /var/www/html/public/index.php(5): Directus\Application\Application->run [] []
hemratna commented 4 years ago

@blob100 I fixed this issue with PR #1764. Please check it and if issue is solved then close it.