Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Editing text in a Slack text+file message fails: "Multiple rows were not expected" #486

Open
jaller94 opened this issue Sep 11, 2020 · 2 comments · May be fixed by #782
Open

Editing text in a Slack text+file message fails: "Multiple rows were not expected" #486

jaller94 opened this issue Sep 11, 2020 · 2 comments · May be fixed by #782
Labels
hacktoberfest Good issues for people to tackle as part of hacktoberfest substitutions An issue relating to translation of event content between Slack and Matrix. T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. Z-Good-First-Issue Good for newcomers

Comments

@jaller94
Copy link
Contributor

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Use PostgreSQL
  2. Go to Slack
  3. Post a message with a file.
  4. Change the text part of the message.

Expected behavior
The message edit will be bridged to Matrix.

Actual behavior
The message edit is not bridged to Matrix because of a bridge error when querying the database.

Screenshots

Sep-11 19:51:58.515 INFO bridge [-] PUT http://localhost:8008/_matrix/client/r0/rooms/!ZmWyDEBEKfOdXjldOC%3Asynapse-dev.local/send/m.room.message/m1599846718514.2 (@slack_matrix-bridge-dev_U010AAR88B1:synapse-dev.local) Body: "{\"body\":\"blub\",\"format\":\"org.matrix.custom.html\",\"formatted_body\":\"b
Sep-11 19:51:58.554 INFO bridge ::1 - - [11/Sep/2020:17:51:58 +0000] "PUT /transactions/475?access_token=<REDACTED> HTTP/1.1" 200 2 "-" "Synapse/1.15.1"

Sep-11 19:51:58.591 INFO bridge [-] PUT http://localhost:8008/_matrix/client/r0/rooms/!ZmWyDEBEKfOdXjldOC%3Asynapse-dev.local/send/m.room.message/m1599846718514.2 (@slack_matrix-bridge-dev_U010AAR88B1:synapse-dev.local) HTTP 200 "{\"event_id\": \"$5RZEYQrnln38cMM7F8sfmQDs1C0M7a4tu7BZ_PUOvkQ\"}"
Event sent to !ZmWyDEBEKfOdXjldOC:synapse-dev.local with event id $5RZEYQrnln38cMM7F8sfmQDs1C0M7a4tu7BZ_PUOvkQ
Sep-11 19:51:58.623 INFO bridge ::1 - - [11/Sep/2020:17:51:58 +0000] "PUT /transactions/476?access_token=<REDACTED> HTTP/1.1" 200 2 "-" "Synapse/1.15.1"

Sep-11 19:52:05.625 INFO SlackGhost Updating user information for U010AAR88B1
Sep-11 19:52:05.634 ERROR BridgedRoom Failed to process event
Sep-11 19:52:05.635 ERROR BridgedRoom QueryResultError {
    code: queryResultErrorCode.multiple
    message: "Multiple rows were not expected."
    received: 2
    query: "SELECT * FROM events WHERE slackChannel = 'C011698KQUC' AND slackTs = '1599846717.004700'"
}

Slack

Screenshot_2020-09-11_20-01-15

Matrix

Screenshot_2020-09-11_20-01-35
"blub" should have been changed to "make" as seen above.
@jaller94 jaller94 changed the title Editing text in a Slack text+file message: "Multiple rows were not expected" Editing text in a Slack text+file message fails: "Multiple rows were not expected" Sep 11, 2020
@Half-Shot
Copy link
Contributor

So this is part of a larger bug, where we sometimes insert multiple rows for a slack message (or a matrix message) in the events table. There should be a 1:1 mapping and ideally we should be enforcing this with constraints. The problem is when there are multiple results, the code that depends on this fall over.

So the fix I'd be looking for would:

  • Remove duplicate mappings in the DB, as part of a schema migration. (perhaps keep the latest).
  • Add constraints to the table which enforces a 1:1 mapping of events. Then, even if we try to insert duplicates it will error or overwrite.

@jaller94 jaller94 self-assigned this Sep 14, 2020
@jaller94 jaller94 removed their assignment Sep 22, 2020
@jaller94 jaller94 mentioned this issue Sep 22, 2020
Closed
@Half-Shot Half-Shot reopened this Sep 22, 2020
@jaller94 jaller94 mentioned this issue Sep 24, 2020
Merged
@Cadair Cadair added the hacktoberfest Good issues for people to tackle as part of hacktoberfest label Sep 28, 2020
@matrix-org matrix-org deleted a comment from Half-Shot Sep 28, 2020
@Cadair Cadair added the substitutions An issue relating to translation of event content between Slack and Matrix. label Apr 7, 2021
@jaller94 jaller94 added T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. Z-Good-First-Issue Good for newcomers and removed bug labels May 16, 2022
@tadzik
Copy link
Collaborator

tadzik commented Apr 4, 2024

Having tested it just now it showed no error, but instead replaced the attachment with the new text and left the old text event intact. Arguably even worse /o\

@tadzik tadzik linked a pull request Apr 5, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
hacktoberfest Good issues for people to tackle as part of hacktoberfest substitutions An issue relating to translation of event content between Slack and Matrix. T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems. Z-Good-First-Issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support editing/removing multipart Slack messages tadzik/matrix-appservice-slack
4 participants
@tadzik @Cadair @Half-Shot @jaller94