-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: increase the DNS allowed response size to solve Data too long for column 'dns_last_result'
#4929
base: master
Are you sure you want to change the base?
Conversation
Some DNS servers might reply with several IP addresses. This avoids errors like the following: ``` Failing: UPDATE `monitor` SET dns_last_result = 'Records: X' WHERE id = 1 - Data too long for column 'dns_last_result' at row 1 ```
Can you provie an entry to reproduce this with? |
@@ -86,7 +86,7 @@ async function createTables() { | |||
table.text("accepted_statuscodes_json").notNullable().defaultTo("[\"200-299\"]"); | |||
table.string("dns_resolve_type", 5); | |||
table.string("dns_resolve_server", 255); | |||
table.string("dns_last_result", 255); | |||
table.string("dns_last_result", 2000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are changing an already existing migration. Please add a new migration instead.
Currently, this does not fix the issue for others due to these lines not gettig applied.
I also don't think that this solves the problem of too large results getting stored. => please truncate what we try to store here instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a new migration instead.
This was the first thing I did on my side. Added a file called 2024-07-11-0000-dns-results.js
with the following content:
exports.up = function (knex) {
return knex.schema.table('monitor', function (table) {
table.string('dns_last_result', 2000).alter();
});
};
exports.down = function (knex) {
return knex.schema.table('monitor', function (table) {
table.string('dns_last_result', 255).alter();
});
};
But this code did not get executed: https://github.com/louislam/uptime-kuma/blob/master/server/database.js#L393-L396, it seems it just happens when the DB is initialized, not every time the server start (or by a trigger):
Line 1671 in 1a5a1a6
await Database.patch(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI @CommanderStorm, the migration issue I'm facing is using mariadb
connection type
@CommanderStorm Surely, you can test with |
Data too long for column 'dns_last_result'
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma
Tick the checkbox if you understand [x]:
Description
Some DNS servers might reply with several IP addresses. This avoids errors like the following:
Type of change
Please delete any options that are not relevant.
Checklist