search

alphapapa / ement.el

A Matrix client for GNU Emacs
GNU General Public License v3.0
488 stars 44 forks source link

Unreadable images aren't handled properly #147

Closed jgarte closed 1 year ago

jgarte commented 1 year ago

Hi,

I'm not sure what I should call this error but ement keeps complaining about the below.

I turned on toggle-debug-on-error and this is what I got.

WDYT

Feel free to rename this issue to something more pertinent.

Debugger entered--Lisp error: (wrong-type-argument consp nil)
  image--set-property(nil :max-height 20)
  ement--resize-image(nil nil 20)
  ement-room-list-column-format-đŸ±([... ...] 1)
  #f(compiled-function (item depth column-name) #<bytecode -0x5e3fdd613bcc93e>)([... ...] 1 #("đŸ±" 0 1 ...))
  #f(compiled-function (depth item) #<bytecode 0x1f279587fc288917>)(1 [... ...])
  #f(compiled-function (depth taxy) #<bytecode -0xa8f587978312ad5>)(1 #s(taxy-magit-section :name "[Left]" :description nil :key "[Left]" :items ... :taxys nil :predicate #f(compiled-function (item) #<bytecode 0x1db0ce3e69be16e9>) :then ignore :make #f(compiled-function (&rest args) #<bytecode -0x1efba48c30aa6cfb>) :take nil :visibility-fn taxy-magit-section-visibility :heading-face-fn #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_10> :level-indent 2 :item-indent 2 :format-fn #f(compiled-function (item) #<bytecode 0x1d62229d113260a9>)))
  #f(compiled-function (depth taxy) #<bytecode -0xa8f587978312ad5>)(0 #s(taxy-magit-section :name "Ement Rooms" :description nil :key nil :items nil :taxys ... :predicate identity :then ignore :make #f(compiled-function (&rest args) #<bytecode -0x1efba48c30aa6cfb>) :take ... :visibility-fn taxy-magit-section-visibility :heading-face-fn #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_10> :level-indent 2 :item-indent 2 :format-fn #f(compiled-function (item) #<bytecode 0x1d62229d113260a9>)))
  taxy-magit-section-format-items((... "Name" ... "Latest" "Topic" "Members" ... ... "Session") (... ... ... ... ... ... ... ... ...) #s(taxy-magit-section :name "Ement Rooms" :description nil :key nil :items nil :taxys ... :predicate identity :then ignore :make #f(compiled-function (&rest args) #<bytecode -0x1efba48c30aa6cfb>) :take ... :visibility-fn taxy-magit-section-visibility :heading-face-fn #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_10> :level-indent 2 :item-indent 2 :format-fn #f(compiled-function (item) #<bytecode 0x1d62229d113260a9>)))
  ement-room-list(:display-buffer-action nil)
  ement-room-list-revert(nil nil)
  revert-buffer()
  ement-room-list-auto-update(#s(ement-session :user ... :server ... :token "syt_amdhcnRl_K..." :transaction-id 3562914184 :rooms ... :next-batch "s3967123296_75..." :device-id "bc49b85568a24a..." :initial-device-display-name "Ement.el: jgar..." :has-synced-p t :account-data ... :events #<hash-table equal 3603/3694 0x265bef9>))
  run-hook-with-args(ement-room-list-auto-update #s(ement-session :user ... :server ... :token "syt_amdhcnRl_K..." :transaction-id 3562914184 :rooms ... :next-batch "s3967123296_75..." :device-id "bc49b85568a24a..." :initial-device-display-name "Ement.el: jgar..." :has-synced-p t :account-data ... :events #<hash-table equal 3603/3694 0x265bef9>))
  ement--sync-callback(#s(ement-session :user ... :server ... :token "syt_amdhcnRl_K..." :transaction-id 3562914184 :rooms ... :next-batch "s3967123296_75..." :device-id "bc49b85568a24a..." :initial-device-display-name "Ement.el: jgar..." :has-synced-p t :account-data ... :events #<hash-table equal 3603/3694 0x265bef9>) (... ... ... ... ...))
  apply(ement--sync-callback (... ...))
  #f(compiled-function (&rest args2) #<bytecode -0xade02cd2fbadcf0>)(((next_batch . "s3967123296_757284974_5097413_2106766452_213471036...") (device_one_time_keys_count (signed_curve25519 . 0)) (org.matrix.msc2732.device_unused_fallback_key_types . []) (device_unused_fallback_key_types . []) (rooms (join (!ZSyPiftXUdlyuPIrKs:ponies.im (timeline (events . []) (prev_batch . "m3967123010~37.3967123073~2.3967123077~1.396712306...") (limited . :json-false)) (state (events . [])) (account_data (events . [])) (ephemeral (events . [...])) (unread_notifications (notification_count . 0) (highlight_count . 0)) (summary))))))
  #f(compiled-function () #<bytecode -0x187d8df3943d8c7c>)()
  plz--sentinel(#<process plz-request-curl> "finished\n")
funcall-interactively: Text is read-only
Quit
QuitPNG warning: iCCP: known incorrect sRGB profile
ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
Wrote /home/jgart/.cache/emacs/recentf
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
Quit [2 times]
ement--resize-image: Wrong type argument: consp, nil
Quit [2 times]
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
ement--resize-image: Wrong type argument: consp, nil [2 times]
Quit [2 times]
Mark set
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
ement--resize-image: Wrong type argument: consp, nil
Mark set
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
ement--resize-image: Wrong type argument: consp, nil [10 times]
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
error in process sentinel: ement--resize-image: Wrong type argument: consp, nil
error in process sentinel: Wrong type argument: consp, nil
Debug on Error enabled globally
Entering debugger...
ement--resize-image: Wrong type argument: consp, nil [2 times]
x is undefined
Mark set
ement--resize-image: Wrong type argument: consp, nil [2 times]
Copied text from "Debugger entered--Lisp error: (wrong-typ"
ement--resize-image: Wrong type argument: consp, nil [7 times]
Wrote /home/jgart/.cache/emacs/recentf
ement--resize-image: Wrong type argument: consp, nil [2 times]
Error running timer ‘battery-update-handler’: (wrong-type-argument consp nil)
ement--resize-image: Wrong type argument: consp, nil
debugger-frame-number: This line is not a function call
ement--resize-image: Wrong type argument: consp, nil
debugger-frame-number: This line is not a function call
ement--resize-image: Wrong type argument: consp, nil [4 times]
Mark set
alphapapa commented 1 year ago

Hello,

Thanks for reporting this problem.

  1. What version of Emacs do you have, and where did the build come from?
  2. What version of Ement do you have?
  3. Does the problem happen if you run Ement in a clean Emacs config, e.g. with https://github.com/alphapapa/with-emacs.sh ?
jgarte commented 1 year ago
  1. GNU Emacs 29.0.90 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37, cairo version 1.16.0)
  2. 0.8.3
  3. I'll get back you on this one as I'll have to find the time to read the setup for with-emacs.sh and test it.
alphapapa commented 1 year ago

Thanks.

  1. GNU Emacs 29.0.90 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37, cairo version 1.16.0)

Do you have a version of Emacs 28 or 27 to test with? Emacs 29 is not yet released, so it would be helpful to know whether we're looking at a change in Emacs 29 or something else.

2. [ 0.8.3](https://toys.whereis.%E3%81%BF%E3%82%93%E3%81%AA/?search=emacs-ement)

Is that the same as this? https://packages.guix.gnu.org/packages/emacs-ement/

3. I'll get back you on this one as I'll have to find the time to read the setup for `with-emacs.sh` and test it.

Basically you would run it like this:

$ with-emacs.sh -i ement

Then try to connect and use the client as you normally do.

jgarte commented 1 year ago

Do you have a version of Emacs 28 or 27 to test with?

Yep.

Is that the same as this? https://packages.guix.gnu.org/packages/emacs-ement/

Yep.

Basically you would run it like this:

Thanks for the TLDR. Much appreciated!

Give me a few days to do more testing and I'll get back here.

jgarte commented 1 year ago

Then try to connect and use the client as you normally do.

By try to connect you mean to call emacsclient with some flag to connect to the frame?

jgarte commented 1 year ago

BTW,

This error still occurs on emacs 28

error in process sentinel: Wrong type argument: consp, nil

GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37, cairo version 1.16.0)

I got the above 28.2 with Guix and ement is same version as before.

alphapapa commented 1 year ago

Then try to connect and use the client as you normally do.

By try to connect you mean to call emacsclient with some flag to connect to the frame?

No, I mean to use Ement ("the [Matrix] client") as you normally do. :)

Please try running this commit: https://github.com/alphapapa/ement.el/commit/c55bc0b362eab1d4d2db495c1ed285ea264295a6 When encountering one of the rooms whose avatar is unexpectedly nil, it should display a warning with the room struct. With that information, we should be able to dig further and find out why the image is ending up nil. Depending on the room, the struct could be a large amount of data. Likely, we'll just need to get the room ID, and then add more debugging code to show what goes wrong with that room's avatar event.

Thanks for your help.

alphapapa commented 1 year ago

Also, for future reference, this should have been solved in https://github.com/alphapapa/ement.el/issues/45, but...

jgarte commented 1 year ago

I ran this in a terminal:

$ guix shell emacs-next emacs-ement --with-commit=emacs-ement=c55bc0b362eab1d4d2db495c1ed285ea264295a6

The above command install emacs 29 and emacs-ement from commit c55bc0b362eab1d4d2db495c1ed285ea264295a6 in a shell environment.

I then, launched Emacs and executed ement-connect and voilĂ , it runs smoothly.

The error completely went away and ement not runs as smooth as butter which I hadn't experienced yet :)

I do have this new warning in *Messages* but maybe ignore or we should investigate?

PNG warning: iCCP: known incorrect sRGB profile [2 times]

jgarte commented 1 year ago

I'll make sure to update the Guix package once this fix is in master.

alphapapa commented 1 year ago

Well, that commit isn't a fix, rather it adds debugging code that should allow us to locate the problem and then fix it in the right place. As you can see in the patch, it should display a warning like Ement room avatar has no image... that should have more information about the room with the problematic avatar. Do you not see a *Warnings* buffer with a message like that?

ement no[w] runs as smooth as butter which I hadn't experienced yet :)

Yes, that's because the error is ultimately signaled from the process sentinel, which Emacs handles by pausing for 2 seconds. I've recently learned that signaling errors from sentinels is bad, so a future version of plz should solve that (i.e. those errors wouldn't cause Emacs to pause). But this error relating to a corrupt room avatar still needs to be fixed in and of itself.

jgarte commented 1 year ago

Oh yeah, looking at the *Warnings* buffer now. It has a ton of info. Looks like some of it might be sensitive information...

Do you need any info from it?

jgarte commented 1 year ago

Here's a snippet of the first couple of characters in the *Warnings* buffer:

⛔ Warning (emacs): Ement room avatar has no image

alphapapa commented 1 year ago

The plan would be for you to give me the room ID (a string like !raNdOmLetteRs11234:matrix.org), then I could add another check for that ID elsewhere in the code, and then get a URL to the avatar image, and then download it and see if it is corrupt or if Emacs can't handle it, etc.

jgarte commented 1 year ago

I saw a room ID like that in *Warnings*. I'll get back to you on that one by this Weekend. Heading out for the night now.

alphapapa commented 1 year ago

Thanks. I pushed another commit to the branch wip/147 that moves the warning to where the avatar is downloaded rather than each time the room list is updated. It also prints only the room ID and the URL to the avatar. Please try that branch when you have time again.

jgarte commented 1 year ago

Here ya go:

⛔ Warning (comp): /gnu/store/y2kz3l34b4dm0ygjz8gk3fhx4fbb3nhr-emacs-feature-loader-1.0.0/share/emacs/site-lisp/feature-loader-1.0.0/feature-loader.el: Error: Symbol's function definition is void all-the-icons-faicon
⛔ Warning (comp): ement-lib.el:99:2: Warning: defconst `ement--color-luminance-dark-limit' docstring has wrong usage of unescaped single quotes (use \= or different quoting)
⛔ Warning (comp): ement-lib.el:130:14: Warning: reference to free variable ‘ement--color-luminance-dark-limit’
⛔ Warning (comp): ement-lib.el:927:40: Warning: avoid `lsh'; use `ash' instead
⛔ Warning (comp): ement-lib.el:928:40: Warning: avoid `lsh'; use `ash' instead
⛔ Warning (comp): ement-lib.el:1142:7: Warning: ‘button-buttonize’ is an obsolete function (as of 29.1); use ‘buttonize’ instead.
⛔ Warning (comp): ement-room.el:2512:4: Warning: docstring wider than 80 characters
⛔ Warning (comp): ement-room.el:2653:14: Warning: ‘point’ is an obsolete generalized variable; use ‘goto-char’ instead.
⛔ Warning (comp): ement-room.el:3533:40: Warning: avoid `lsh'; use `ash' instead
⛔ Warning (comp): ement-room.el:3534:40: Warning: avoid `lsh'; use `ash' instead
⛔ Warning (ement): Room avatar seems unreadable:  ROOM-ID:"!yAXokUArOJxHEACeib:pussthecat.org"  AVATAR-URL:"https://matrix-client.matrix.org/_matrix/media/r0/download/snopyta.org/rnKrxCoRJcAiGXfZVaDEfUUS"
⛔ Warning (comp): ement-room-list.el:161:2: Warning: defvar `ement-room-list-keys' docstring wider than 80 characters

or screenshot on a different run:

image

WDYT

jgarte commented 1 year ago

The above was tested in a guix shell on the latest commit from wip/147:

> guix shell emacs-next emacs-ement --with-branch=emacs-ement=wip/147 --check
updating checkout of 'https://github.com/alphapapa/ement.el'...
retrieved commit a31cbeefca53bc065c8e637849caa2c90012a43f
guix shell: checking the environment variables visible from shell '/gnu/store/qvj03fib58k6ldcv3pas3523mz3baiic-zsh-5.8.1/bin/zsh'...
guix shell: All is good!  The shell gets correct environment variables.
alphapapa commented 1 year ago

Ok, I think I see the problem: When I open https://matrix-client.matrix.org/_matrix/media/r0/download/snopyta.org/rnKrxCoRJcAiGXfZVaDEfUUS in Firefox, or download it with wget, the server says it's invidious.png, but Firefox and the file command identify it as MS Windows icon resource - 3 icons, 48x48, 32 bits/pixel, 32x32, 32 bits/pixel. And when I feed that file data into Emacs's create-image, it returns nil. And the code in ement-room-list.el expects that if the avatar image was downloaded by the avatar event handler, it will be non-nil.

And apparently Emacs does not support ICO files at all, as (image-type-from-file-name "image.ico") returns nil.

So the "fix" in Ement will be basically what that branch does now, to show a warning when the avatar is downloaded and found unreadable, and to leave the room's avatar slot properly nil.

In the meantime, you might want to advise the room's admin that they should replace the avatar with a real PNG image.

Thanks for your help with this error. I'll release the fix in v0.9, since it's basically ready to tag anyway, I think.