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.

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

Error: Improper store into indexable object #1043

Closed
ghost opened this issue Nov 8, 2018 · 29 comments
Closed

Error: Improper store into indexable object #1043

ghost opened this issue Nov 8, 2018 · 29 comments
Assignees
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Nov 8, 2018

icomoon.zip

Hello,

I have made a library which looks like this :

WAFileLibrary subclass: #PaintingLibrary
instanceVariableNames: ''
classVariableNames: ''
package: 'Painting'

then I did these steps on the web interface to add a css file

  1. open localhost and choose for config and then files
  2. then choose the library and choose configure.
  3. then browse to choose the file and then add.

then I see this stack trace.

Seaside Walkback
Error: Improper store into indexable object

Debug Full Stack
Stack Trace

thisContext
    ByteString(Object)>>error:
self
    '...etc...

thisContext
    ByteString(Object)>>errorImproperStore
self
    '...etc...

thisContext
    ByteString>>at:put:
self
    '...etc...

thisContext
    WriteStream>>nextPut:
self
    a WriteStream

thisContext
    [ :writeStream | | readStream | readStream := aString readStream. [ readStream atEnd ] whileFalse:...etc...
self
    a GRPharoPlatform

I work on Pharo 6.1 on Windows 10.

@marschall
Copy link
Contributor

Which version of Seaside do you use?

@ghost
Copy link
Author

ghost commented Nov 9, 2018

I installed yesterday the latest stable one. I do not know how I can see which version I have installed
I installed Seaside3.

@ghost
Copy link
Author

ghost commented Nov 9, 2018

and I work on a PharoWeb image downloaded from the seaside page.
if I want I can include the image here

@marschall
Copy link
Contributor

Can you describe the installation steps you took? Did you use this script?
https://github.com/SeasideSt/Seaside#install-in-pharo

@ghost
Copy link
Author

ghost commented Dec 18, 2018

I will try it
I did at first from the Catalog

@ghost
Copy link
Author

ghost commented Dec 18, 2018

Now trying that script on a P7-rc1 image

@ghost
Copy link
Author

ghost commented Dec 18, 2018

Then no problems at all

@SabineMa
Copy link

SabineMa commented Dec 18, 2018 via email

@theseion
Copy link
Member

Thanks Sabine, that looks like it could be related.

@theseion theseion self-assigned this Dec 22, 2018
@theseion theseion added the bug label Dec 22, 2018
@theseion
Copy link
Member

The error is caused by GRPlatform>>convertToSmalltalkNewlines:, which expects a string as input. The input will only be a string though if the content type of the file has a character set:

GRPlatform current convertToSmalltalkNewlines: (aFile contentType charSet isNil
	ifTrue: [ aFile rawContents ]
	ifFalse: [ aFile contentsDecoded ])

When the input is not a string (but a byte array) the conversion will fail because the tokens are bytes but the write stream uses a string collection.

I don't know why the conversion is there but I guess someone had a reason to add it. So my suggestion is to only convert files whose content can be interpreted as a string (that works experimentally by doing a string type check).

@jbrichau @marschall Maybe you can fill in the blanks here?

@SabineMa Which method in which package were you referring to? I didn't find that comment anywhere.

@jecisc
Copy link
Member

jecisc commented Dec 23, 2018

It was my fix in the project pharo-contributions/Artefact in PDFDemos class >> streamNamed:

@theseion
Copy link
Member

So it's not related then. Thanks.

@SabineMa
Copy link

Something new here? I have similar problems like ghost when moving to Pharo 7 and I can not use my less solution (I use a subclass of WAFileLibrary to add a less file which is then compiled by a less compiler). Would be nice if this could be fixed. I can test it but I dont know how to fix it...

ByteArray(Object)>>errorImproperStore
ByteArray(Object)>>at:put:
ReadWriteStream(WriteStream)>>nextPut:
[ :v | self nextPut: v ] in ReadWriteStream(Stream)>>nextPutAll: in Block: [ :v | self nextPut: v ]
ByteString(SequenceableCollection)>>do:
ReadWriteStream(Stream)>>nextPutAll:
ReadWriteStream(WriteStream)>>nextPutAll:
WABufferedResponse(WAResponse)>>nextPutAll:
WABufferedResponse(WAResponse)>>document:
WABufferedResponse(WAResponse)>>document:mimeType:
[ :response |
response
cacheFor: self cacheDuration;
document: (self documentOf: selector)
mimeType: (self mimetypeOf: selector) ] in RKALibrary(WAFileLibrary)>>handle: in Block: [ :response | ...
WARequestContext>>respond:
RKALibrary(WAFileLibrary)>>handle:
RKALibrary class(WAAbstractFileLibrary class)>>handle:
WAFileHandler>>responseForContext:
WAFileHandler>>handleFiltered:
[ self filter handleFiltered: aRequestContext ] in WAFileHandler(WARequestHandler)>>handle: in Block: [ self filter handleFiltered: aRequestContext ]
[ activeProcess psValueAt: index put: anObject.
aBlock value ] in WACurrentRequestContext(DynamicVariable)>>value:during: in Block: [ activeProcess psValueAt: index put: anObject....
BlockClosure>>ensure:
WACurrentRequestContext(DynamicVariable)>>value:during:
WACurrentRequestContext class(DynamicVariable class)>>value:during:
WACurrentRequestContext class(GRDynamicVariable class)>>use:during:
[ WACurrentRequestContext use: self during: aBlock ] in WARequestContext>>push:during: in Block: [ WACurrentRequestContext use: self during: aBlock...etc...
BlockClosure>>ensure:
WARequestContext>>push:during:
WAFileHandler(WARequestHandler)>>handle:
WADispatcher>>handleFiltered:named:
WADispatcher>>handleFiltered:
[ self filter handleFiltered: aRequestContext ] in WADispatcher(WARequestHandler)>>handle: in Block: [ self filter handleFiltered: aRequestContext ]

bildschirmfoto 2019-02-12 um 15 58 13

@SabineMa
Copy link

I fixed it temporarily by sending >>asByteArray to the string (in my case to mainStylesLess) in case of Pharo 7

@jbrichau
Copy link
Member

@SabineMa could you provide me the file you try to upload (via email if you do not want it to be public) and a brief description how I can reproduce? The original poster @ghost reported the problem went away once he loaded the most recent version of Seaside, so this was not investigated further.

If I can reproduce, I can have a look to fix.

@SabineMa
Copy link

Hi @jbrichau ,

I hope you can reporduce it with this:

  • start with launcher a pharo 7.0 64 bit image
  • install seaside3 from launcher
  • file in this files
  • register application and start in browser
  • start server and go to http://localhost/RKX
  • put a halt in errorImproperStore so you can better see the source of the error

I hope this works if not please tell me

RKATestViewLibrary.st.txt
RKATestLibrary.st.txt

@jbrichau
Copy link
Member

@SabineMa What do you mean with 'install seaside3 from launcher' ? Do you install Seaside with the instructions in https://github.com/SeasideSt/Seaside#install-in-pharo ?

@SabineMa
Copy link

@jbrichau I do not load seaside directly, I load MDL in my baseline:

spec baseline: 'MaterialDesignLite' with: [ spec repository: 'github://DuneSt/MaterialDesignLite:v2.0.0/src' ]

The statement "* install seaside3 from launcher" was only for reproducing the bug in a new image. But yesterday when I wrote the above, (it was late and I had to go to bed :-) ), I did not finish the reproduce test in the new image.

Can you reproduce the bug with the two fileouts?

@jbrichau
Copy link
Member

Hi @SabineMa
Sorry it took so long, but I executed the steps you mention and I am not seeing an error.

I see 'hello world' in my browser when I open the test app.
Are you still able to reproduce this?

@SabineMa
Copy link

Hi Johan,
I checked and yes, I can reproduce it. I took a Pharo7-64 bit image, loaded seaside3 from catalog, and filed the files in, then register the application. One can see the error in the browser like in the screenshot below or when adding a halt in Object>>errorImproperStore.
Bildschirmfoto 2019-08-28 um 14 51 59

I can show it to you here at ESUG.

@jbrichau
Copy link
Member

Ow. From the catalog?
I dont think that is latest Seaside. Can you use the load instructions from github?

@SabineMa
Copy link

loading with

Metacello new
 baseline:'Seaside3';
 repository: 'github://SeasideSt/Seaside:master/repository';
 load

does also bring the error. Note that the page displays as in the screenshot. You have to enter the halt as described to see the walkback.

@jbrichau
Copy link
Member

@SabineMa Ok, I can reproduce it now. Thanks!

@jbrichau
Copy link
Member

jbrichau commented Aug 29, 2019

Ok, the '.less' file fails to get served by WAFilelibrary because it fails to identify it's mimetype as text, defaulting to a binary stream in the response, while the contents of the method is a String. As a result, it tries to put a Character in a ByteArray,

The easy fix here is to include the 'less' extension in the list of mimetypes so it gets set to 'text/plain' and the error goes away.

In general, it is going to pop up for any mimetype we do not properly list in WAFileLibrary. The default mimetype is binary: should we not default to plain text if the contents of the method is a String and not a ByteArray?

@jbrichau
Copy link
Member

@SabineMa
I think you have added the testStyles.less file manually to RKATestLibrary because if you would have loaded it using RKATestLibrary addFileAt: 'testStyles.less', it would have also added the file as binary to the file library and served it appropriately.

Now, it makes sense to detect .less file extensions as text, so I will add that anyway.

@SabineMa
Copy link

SabineMa commented Aug 29, 2019

@jbrichau yes I add it manually with:

anHtmlRoot link
		url: RKATestLibrary / #testStylesLess;
		relationship: 'stylesheet/less';
		beCss.

thank you for fixing it

@jbrichau
Copy link
Member

@SabineMa I meant adding it using copy/paste to the class, rather than loading it with the snippet I showed. Anyway, it should be fixed in 3.4.0 and if you want it earlier, you can apply the fix of #1171 to your codebase.

@jbrichau
Copy link
Member

@theseion I don't think it makes sense to use convertToSmalltalkNewlines: in the case of a binary file upload. So my thought is to only do that when we can decode the contents (in the ifFalse: branch).

Now, I just wonder if we should not expect to have charSet when someone is uploading a css file from the browser...

@SabineMa
Copy link

@jbrichau I did not copy neither use addFileAt:
I keep my css in methods and the file is generated dynamically :-)
This is just for information.
For my, everything is fine, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants