search

sugarlabs / sugar-web

Components for Sugar web activities
Apache License 2.0
13 stars 32 forks source link

FindWords-4 causes error in webactivity.py #135

Open quozl opened 4 years ago

quozl commented 4 years ago 
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/sugar3/activity/webactivity.py", line 166, in _app_scheme_cb
    request.finish(Gio.File.new_for_path(path).read(None),
gi.repository.GLib.Error: g-io-error-quark: Error opening file /usr/share/sugar/activities/FindWords.activity/%22activity:/org.sugarlabs.FindWords/activity/icon.svg%22: No such file or directory (1)
Saumya-Mishra9129 commented 4 years ago

@quozl I am not able to find this error. Can you please tell steps of reproduction? But I faced same error while running https://github.com/llaske/PhysicsJS.activity

quozl commented 4 years ago

On Ubuntu 20.04, install Sugar 0.117 using the sucrose package, install FindWords-4 using the sugar-findwords-activity package, log in to Sugar, start the FindWords activity, stop the activity, log out from Sugar, and check the Sugar logs.

It may not be necessary to reproduce the error in order to fix it. Human static analysis of the source code may be sufficient.

Saumya-Mishra9129 commented 4 years ago

@srevinsaju Do you think error can be due to same cause as introduced in https://github.com/sugarlabs/pukllanapac/issues/12 ?

srevinsaju commented 4 years ago

@Saumya-Mishra9129 I can say that it is a similar error to FractionBounce and Pulkanapac, but not the same. The error code provided is different. In Pulkanapac and Fraction Bounce, it was (2), but the cause was two different things things. But in this case the error is (1), I haven't gone througgt the activity or a similar error in the past, need to debug why it happens. (maybe it's because of a particular version of librsvg, and hence you cannot reproduce?)

quozl commented 4 years ago

1 is EPERM, permission denied.

2 is ENOENT, no such file or directory.

You can get these using import errno and errno.errorcode[1]. See "man errno" at bash prompt for more detail. It's a C thing. Learn C, you'll need it.

quozl commented 4 years ago

@Saumya-Mishra9129, I'll give you a bit of mentoring; help or teach student how to solve difficult technical problems.

Look carefully at the traceback, as it contains a lot of information;

From that we might conclude;

My guess is that WebKit2 API has changed.

Perhaps the solution is to remove the quotes, remove the bundle_id, and join the result. But why would that be the solution?

srevinsaju commented 4 years ago

@quozl does the %22 have any significance here?, Maybe the %22 was an unneeded character and hence the file was not been able to be found. The Error (1) related to Permission might be because it was installed to /usr we might get a different answer if it's installed to ~/Activities and know what the real cause is.

quozl commented 4 years ago

@srevinsaju, %22 is an HTML entity encoded double-quote character, in the byte position 0x22 in the ASCII and UTF-8 character sets. I don't know why it is there. If it was b%22 then I'd suggest some problem with bytes and strings, but it isn't. A next step is to find out from the source code of WebKit or the activity why the value is arriving with quotes. If it happens with other activities, it could be a mistake that was copied. If it happens with the hello web activity as well, then perhaps there is a mistake in sugar-web library used in all activities. Otherwise I'd blame WebKit. It may be a feature of WebKit that doesn't get much attention, and we might be the first to notice.

This brings up a thing about speculation; that it is rarely useful except to guide where to look for evidence.

quozl commented 4 years ago

Correction. It looks like RFC 1738 encoding, as used by HTML form submission.

Saumya-Mishra9129 commented 4 years ago

What I get about RFC 1738 encoding-

The URL specification RFC 1738 specifies that only a small set of characters can be used in a URL. Those characters are: A to Z (ABCDEFGHIJKLMNOPQRSTUVWXYZ) a to z (abcdefghijklmnopqrstuvwxyz)

Correction. It looks like RFC 1738 encoding, as used by HTML form submission.

Why you come to this correction??

quozl commented 4 years ago

See RFC 1738 section 2.2 "URL Character Encoding Issues" and also section 5 BNF where the syntax is reinforced.

If not source analysis, I suggest interactive debugging of the code path that leads to our callback being called. Being able to step through WebKit and seeing the process steps will be important to diagnosing this.

Saumya-Mishra9129 commented 4 years ago

I tried a little debugging today tried adding a line , to parse a url in toolkit here https://github.com/sugarlabs/sugar-toolkit-gtk3/blob/8c976bd56a488d5612b1f69b24c45af114219079/src/sugar3/activity/webactivity.py#L162

I tried this 

import urllib.parse
def _app_scheme_cb(self, request, user_data):
        path = os.path.join(self._bundle_path,
                            os.path.relpath(request.get_path(), "/"))
        path = urllib.parse.unquote(path).replace('"','')
        request.finish(Gio.File.new_for_path(path).read(None),
                       -1, Gio.content_type_guess(path, None)[0])

Got this error again but without encoded url, seems like we were wrong, %22 had nothing to do with error, it is something different.

Screenshot from 2020-06-09 13-21-05

quozl commented 4 years ago

Next time please copy and paste the message into the GitHub issue; adding a screenshot makes it impossible to find it again by searching.

We never said %22 was the cause of the problem. I had said above

Perhaps the solution is to remove the quotes, remove the bundle_id, and join the result. But why would that be the solution?

You've done the first step. The file is still not found. And we still don't know why this has to be done. Why did an older version provide the file path in a different form, and what was that form? How can we know which form it will be in?

Saumya-Mishra9129 commented 4 years ago

Why did an older version provide the file path in a different form, and what was that form? How can we know which form it will be in?

The above error leads to following traceback

Traceback (most recent call last):
   File "/usr/lib/python3/dist-packages/gwebsockets/server.py", line 127, in _message_write_cb
      written = stream.write_bytes_finish(result)
gi.repository.GLib.Error: g-io-error-quark: Error sending data: Broken pipe (44)

It can be seen after logout and login again -> check previous session log files (shell.log)

quozl commented 4 years ago

That's not in the same code path, so why is it relevant? You'd best diagnose that separately. Don't rush to change the code, try to figure out what the traceback means. Not literally, but figuratively.

quozl commented 4 years ago

And if it is not relevant, there's still the problem of making sure the path to the file is complete and correct. Removing the %22 is only the first step of many. And we still don't know why that path is being received now when it was not before. What changed in WebKit?

Saumya-Mishra9129 commented 4 years ago

That's not in the same code path, so why is it relevant? You'd best diagnose that separately. Don't rush to change the code, try to figure out what the traceback means. Not literally, but figuratively

I think It can be relevant, I got that error whenever I open a find-words-activity and logout a session. I am trying to figure out how it is related to prior found traceback.

And if it is not relevant, there's still the problem of making sure the path to the file is complete and correct. Removing the %22 is only the first step of many. And we still don't know why that path is being received now when it was not before. What changed in WebKit?

I don't know. I need to figure that out.

quozl commented 4 years ago

That's not in the same code path, so why is it relevant? You'd best diagnose that separately. Don't rush to change the code, try to figure out what the traceback means. Not literally, but figuratively

I think It can be relevant, I got that error whenever I open a find-words-activity and logout a session. I am trying to figure out how it is related to prior found traceback.

My assessment is that until you've fixed the path enough so that the SVG file is able to be read, which is more than removing %22, this new error unlikely to be relevant. It also happens much later. It also happens in a different code path. I'd say it was just waiting to happen but was prevented previously by the "Error opening file", or that the SVG file was not properly sent back as a reply so the client has abandoned the connection, hence Broken pipe.

shaansubbaiah commented 4 years ago

Tried testing on another Web Activity - Moon, but this occurred only in the Find Words Activity.

I think this has to do with the sugar-web library used in Find Words Activity. The sugar-web library calls webactivity.py. After replacing the sugar-web directory from the Find Words Activity with the one in Moon Activity, the error disappeared.

Looking at the diffs in icon.js, getBackgroundURL() was different and the only instance of icon.svg was in the Activity button's background-image property.

Not sure where the Moon Activity got the modified getBackgroundURL() in icon.js as it is not present in the Sugar Labs sugar-web repository. @quozl there aren't any changes to the icon.js file in the Moon Activity apart from the initial commit, do you know from where it was taken?

Changes that fixed it. (from lib/sugar-web/graphics/icon.js in Moon Activity)


# FindWords.activity/lib/sugar-web/graphics/icon.js

    function getBackgroundURL(elem) {
        var style = elem.currentStyle || window.getComputedStyle(elem, '');
        // Remove prefix 'url(' and suffix ')' before return
-       return style.backgroundImage.slice(4, -1);
+       var res = style.backgroundImage.slice(4, -1);
+       var last = res.length-1;
+       if (res[0] == '"' && res[last] == '"') {
+           res = res.slice(1, last);
+       }
+       return res;
    }

Tested on Sugar Live Build
quozl commented 4 years ago

Thanks, well done.

Moon's icon.js came from Sugarizer. Changes in Sugarizer haven't made their way back to sugar-web yet; it's an opportunity. The change you've highlighted is https://github.com/llaske/sugarizer/commit/379bff4ae96122564d65fbc28bdcab37d78e0ef8#diff-379562b4f0a89909712914626419f509.

So we can move this issue to sugar-web repository.

shaansubbaiah commented 4 years ago

Out of curiosity, would there be any other changes required other than copying the latest version of sugar-web Sugarizer to the Sugar repo? According to the architecture described in the README, it is independent, so just copying would do?

quozl commented 4 years ago

Sorry for delay, notification was filed in spam. Yes, that's the general idea. One would also look for regressions.