Circular error when deserializing drift modules

[andresthinkme]

After upgrading from Drift 2.19.1 to 2.20.2, I get an error message when trying to build.

The error message is
"Could not deserialize DriftElementId(package:axibis_base/database/sales/sales_invoices_dao.dart, sales_invoices)
Internal error while deserializing DriftElementId(package:axibis_base/database/sales/sales_invoices_dao.dart, sales_invoices): Bad state: Circular error when deserializing drift modules. This is a bug in drift_dev!"

When I changeg back to version 2.19.1 this was solved. Nothing changed in my code after the update.

[simolus3]

Hi, the error is not critical (and also likely happened in the previous version, we just didn't log it). Thanks for the report though, this is still something we should fix.
Is it possible for you share the definition of SaleInvoices?

[Dumoulin-Lander]

Hi @simolus3, we get the exact same warning in our code for a bunch of different things. Let me give you an example so you have something to go off of:

[WARNING] drift_dev on lib/src/operator_db/operator_db_base.dart:
Could not deserialize DriftElementId(package:database/src/operator_db/tables/execution_header_additional_info_table.dart, execution_header_additional_infos)
Internal error while deserializing DriftElementId(package:database/src/operator_db/tables/execution_header_action_additional_info_table.dart, execution_header_action_additional_infos): Bad state: Circular error when deserializing drift modules. This is a bug in drift_dev! at 
#0      ElementDeserializer._readElementReference (package:drift_dev/src/analysis/serializer.dart:435:7)
#1      ElementDeserializer._readDriftElement (package:drift_dev/src/analysis/serializer.dart:494:15)
#2      ElementDeserializer.readDriftElement (package:drift_dev/src/analysis/serializer.dart:466:28)
<asynchronous suspension>
#3      ElementDeserializer._readElementReference (package:drift_dev/src/analysis/serializer.dart:443:14)
<asynchronous suspension>
#4      ElementDeserializer._readDriftElement (package:drift_dev/src/analysis/serializer.dart:494:9)
<asynchronous suspension>
#5      ElementDeserializer.readDriftElement (package:drift_dev/src/analysis/serializer.dart:466:22)
<asynchronous suspension>
#6      DriftResolver._restoreOrResolve (package:drift_dev/src/analysis/resolver/resolver.dart:48:16)
<asynchronous suspension>
#7      DriftResolver.resolveReferencedElement (package:drift_dev/src/analysis/resolver/resolver.dart:153:26)
<asynchronous suspension>
#8      LocalElementResolver.resolveDartReferenceOrReportError (package:drift_dev/src/analysis/resolver/resolver.dart:256:9)
<asynchronous suspension>
#9      DartAccessorResolver.resolve (package:drift_dev/src/analysis/resolver/dart/accessor.dart:61:21)
<asynchronous suspension>
#10     DriftResolver._resolveDiscovered (package:drift_dev/src/analysis/resolver/resolver.dart:103:22)
<asynchronous suspension>
#11     DriftResolver._restoreOrResolve (package:drift_dev/src/analysis/resolver/resolver.dart:60:12)
<asynchronous suspension>
#12     DriftResolver.resolveEntrypoint (package:drift_dev/src/analysis/resolver/resolver.dart:42:12)
<asynchronous suspension>
#13     DriftAnalysisDriver.resolveElement (package:drift_dev/src/analysis/driver/driver.dart:269:16)
<asynchronous suspension>
#14     DriftAnalysisDriver._analyzeLocalElements (package:drift_dev/src/analysis/driver/driver.dart:254:7)
<asynchronous suspension>
#15     DriftAnalysisDriver.resolveElements (package:drift_dev/src/analysis/driver/driver.dart:321:5)
<asynchronous suspension>
#16     DriftAnalysisDriver.fullyAnalyze (package:drift_dev/src/analysis/driver/driver.dart:329:19)
<asynchronous suspension>
#17     _DriftBuildRun._analyze (package:drift_dev/src/backends/build/drift_builder.dart:193:20)
<asynchronous suspension>
#18     _DriftBuildRun.run (package:drift_dev/src/backends/build/drift_builder.dart:171:9)
<asynchronous suspension>
#19     DriftBuilder.build (package:drift_dev/src/backends/build/drift_builder.dart:108:5)
<asynchronous suspension>
#20     runBuilder.buildForInput (package:build/src/generate/run_builder.dart:83:7)
<asynchronous suspension>
#21     Future.wait.<anonymous closure> (dart:async/future.dart:534:21)
<asynchronous suspension>
#22     scopeLogAsync.<anonymous closure> (package:build/src/builder/logging.dart:32:40)
<asynchronous suspension>

I've added the table definition below, as a txt file:

execution_header_action_additional_info_table.txt

[simolus3]

Thanks for the details! I suspect this might be related to the ExecutionHeaderAdditionalInfos reference, could you share the definition of that table as well?

[Dumoulin-Lander]

Sure!

execution_header_additional_info_table.txt

[andresthinkme]

Hi @simolus3,

Sorry for the late reply. Here is the definition for the table and the dataclass used for it.

@UseRowClass(SalesInvoice)
class SalesInvoices extends Table {
TextColumn get id => text().clientDefault(() => locator().v4())();
TextColumn get number => text().unique()();
TextColumn get ticketId => text().nullable().references(Tickets, #id)();
TextColumn get paymentMethodId => text().nullable().references(PaymentMethods, #id)();
TextColumn get paymentStatus => textEnum()();
TextColumn get structuredMessage => text().withLength(min: 12, max: 12).nullable()();
TextColumn get customerId => text().nullable().references(Customers, #id)();
TextColumn get customerNumber => text().nullable()();
TextColumn get customerName => text().nullable()();
TextColumn get customerVatNumber => text().nullable()();
TextColumn get customerEmail => text().nullable()();
TextColumn get customerPhone => text().nullable()();
TextColumn get customerAddress => text().nullable()();
TextColumn get customerAddress2 => text().nullable()();
TextColumn get customerPostCode => text().nullable()();
TextColumn get customerCity => text().nullable()();
TextColumn get customerCountry => text().nullable()();
TextColumn get comment => text().nullable()();
DateTimeColumn get dueDate => dateTime().withDefault(currentDateAndTime)();
TextColumn get ourReference => text().nullable()();
TextColumn get yourReference => text().nullable()();
DateTimeColumn get createdAt => dateTime().withDefault(currentDateAndTime)();
TextColumn get syncStatus => textEnum()();

@OverRide
Set get primaryKey => {id};
}

class SalesInvoice implements Insertable {
final String id;
final String number;
final String? ticketId;
final String? paymentMethodId;
final PaymentStatus paymentStatus;
final String? structuredMessage;
final String? customerId;
final String? customerNumber;
final String? customerName;
final String? customerVatNumber;
final String? customerEmail;
final String? customerPhone;
final String? customerAddress;
final String? customerAddress2;
final String? customerPostCode;
final String? customerCity;
final String? customerCountry;
final String? comment;
final DateTime dueDate;
final String? ourReference;
final String? yourReference;
final DateTime createdAt;
// enum
final SyncStatus syncStatus;
}

[Fraa-124]

We're also seeing this issue. It's a warning, but it's causing our CI builds to fail.

[simolus3]

@Dumoulin-Lander You have two tables referencing each other (ExecutionHeaderAdditionalInfos.actionId references ExecutionHeaderActionAdditionalInfos, which references the first table through executionHeaderAdditionalInfoId). Drift's error message here should be much better, but this is a problem in your schema and it can cause issues in the database when foreign keys are enabled (because we can't create either table with a correct foreign key constraint before creating the other one).

@andresthinkme and @Fraa-124, could you also check whether your table might have circular references to other tables linking back to the original table? That would explain the error.
@Fraa-124 I'm surprised that this would fail the build, are you running it in a special mode to fail on warnings?

[dickermoshe]

@simolus3
Why can't we create both tables and then add the foreign key columns afterward.
This is what Django does.

[dickermoshe]

e.g.

from django.db import models

class Student(models.Model):
    name = models.CharField(max_length=100)
    group = models.ForeignKey("StudentGroup", on_delete=models.CASCADE)

class StudentGroup(models.Model):
    name = models.CharField(max_length=100)
    top_student = models.ForeignKey(
        Student, on_delete=models.CASCADE, related_name="top_student"
    )

There is a circular reference here. But here is how django can creates it

BEGIN;
--
-- Create model Student
--
CREATE TABLE "hello_student" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "name" varchar(100) NOT NULL);
--
-- Create model StudentGroup
--
CREATE TABLE "hello_studentgroup" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "name" varchar(100) NOT NULL, "top_student_id" bigint NOT NULL REFERENCES "hello_student" ("id") DEFERRABLE INITIALLY DEFERRED);
--
-- Add field group to student
--
CREATE TABLE "new__hello_student" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "name" varchar(100) NOT NULL, "group_id" bigint NOT NULL REFERENCES "hello_studentgroup" ("id") DEFERRABLE INITIALLY DEFERRED);
INSERT INTO "new__hello_student" ("id", "name", "group_id") SELECT "id", "name", NULL FROM "hello_student";
DROP TABLE "hello_student";
ALTER TABLE "new__hello_student" RENAME TO "hello_student";
CREATE INDEX "hello_studentgroup_top_student_id_dfeeb764" ON "hello_studentgroup" ("top_student_id");
CREATE INDEX "hello_student_group_id_3dd4e5e4" ON "hello_student" ("group_id");
COMMIT;

This looks really complex. I don't know how feasible it would be to bake this into drift, but it's technically possible. even in sqlite

[simolus3]

With the alterTable machinery we already have in place, it shouldn't be too hard to pull off (even though we'll likely create SQL that is even more verbose, haha).

That's something for the future though, the broken state is that we don't currently support circular references (and there are several places in drift_dev that are supposed to detect them and loudly complain, none of them apparently working correctly). That's the part I need to take a look at first - for instance, the default migration logic creating all tables relies on them being topologically sorted at build time which wouldn't work if we silently drop circular references.

[simolus3]

Alright, I've found the root cause but it's not going to be easy to resolve (or at least I don't have an obvious solution). We're running modular analysis to improve build performance, and the tables are defined in different files.

So let's say that we had two files, a.dart and b.dart with tables referencing each other. When drift_dev:analyzer runs on a.dart, it finds the local table, starts resolving it and discovers the reference to the table in b.dart. While attempting to resolve that table, it detects the circular error and logs the error in the definition of table b (but not in table a, because cycles are broken where they are first detected). To avoid logging errors multiple times however, drift_dev:analyzer only logs errors in a.dart (and thinks they aren't any). When running on b.dart, this game repeats (it thinks there's an error in a.dart due to the circular reference but doesn't report this).

We should probably keep track of transitive attempted dependencies somehow to file the error on both tables, which would fix this issue (or at least give it a better error message).

[iampopal]

I am getting the same error any quick solutions.

[WARNING] drift_dev on lib/services/database/pharmacy/drug_formulas.dart:
Could not deserialize DriftElementId(package:system_server/services/database/pharmacy/drug_formulas.dart, drug_formulas)
Internal error while deserializing DriftElementId(package:system_server/services/database/files/server_files.dart, server_files): Bad state: Circular error when deserializing drift modules. This is a bug in drift_dev! at 
#0      ElementDeserializer._readElementReference (package:drift_dev/src/analysis/serializer.dart:435:7)
#1      ElementDeserializer._readDriftElement (package:drift_dev/src/analysis/serializer.dart:494:15)
#2      ElementDeserializer.readDriftElement (package:drift_dev/src/analysis/serializer.dart:466:28)
<asynchronous suspension>
#3      ElementDeserializer._readElementReference (package:drift_dev/src/analysis/serializer.dart:443:14)
<asynchronous suspension>

[simolus3]

Note that this error is mostly harmless and doesn't change the generated code. Could you also check whether the table in drug_formulas.dart references a table that itself then references the drug_formulas.dart table again?

[iampopal]

Thank you so much for quickly replaying. I actually had error of manager and reference need to be added Then updated drift to latest version and it start showing: Circular error when deserializing After seeing this I downgraded back and start seeing @reference required for manager again. Then I read the manager documentation and use cases so I actually do not use it and not need it. So I disabled it from config yml file of drift. Now my issue is solved. Can you please tell me about manager is it required to use or does it helps.

…

On Sun, 6 Oct 2024 at 4:12 PM Simon Binder ***@***.***> wrote: Note that this error is mostly harmless and doesn't change the generated code. Could you also check whether the table in drug_formulas.dart references a table that itself then references the drug_formulas.dart table again? — Reply to this email directly, view it on GitHub <#3227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIUFLXGNT5SHKT4EQKPDQADZ2EO2XAVCNFSM6AAAAABOPJ7WNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGQYDMNRTG4> . You are receiving this because you commented.Message ID: ***@***.***>

[dickermoshe]

You don't need to use it but it's just an easier way to write queries.
What was the manager bug? If there is a bug I want to fix it.

[dickermoshe]

@simolus3 This error message should probably be clarified.

[iampopal]

The manager bug was that it required to add @reference at every relationship I had in latest version. Going back to older versions fixes that.

…

On Sun, 6 Oct 2024 at 8:40 PM Moshe Dicker ***@***.***> wrote: @simolus3 <https://github.com/simolus3> This error message should probably be clarified. — Reply to this email directly, view it on GitHub <#3227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIUFLXDGPPPAQCUCEFRBL2TZ2FOGJAVCNFSM6AAAAABOPJ7WNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGQ4TENZSGA> . You are receiving this because you commented.Message ID: ***@***.***>

[dickermoshe]

Oh, but that would just print a warning, those can be ignored

[iampopal]

But after those warnings the build was not working. New files ware not generating. Doing above steps help solve the problem. Thank you for all you guys being very active and responding.

…

On Sun, 6 Oct 2024 at 10:04 PM Moshe Dicker ***@***.***> wrote: Oh, but that would just print a warning, those can be ignored — Reply to this email directly, view it on GitHub <#3227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIUFLXDHI6WZWYWCQSHMW7DZ2FYCFAVCNFSM6AAAAABOPJ7WNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGUYTSMJYHE> . You are receiving this because you commented.Message ID: ***@***.***>

[dickermoshe]

Would you mind sharing it?
Or at least just the table definitions which were causing this issue?