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

jsgi error module #23

Open
dmachi opened this issue Feb 3, 2013 · 3 comments
Open

jsgi error module #23

dmachi opened this issue Feb 3, 2013 · 3 comments

Comments

@dmachi
Copy link
Contributor

dmachi commented Feb 3, 2013

When trying to throw an AccessError, I ran into a problem at

https://github.com/persvr/pintura/blob/master/jsgi/error.js#L24

Basically, e instanceof AccessError is always false. I'm wondering if it is because they don't come from the same physical file. I load require("perstore/errors").AccessErrors which pulls in ./node_modules/perstore/errors.js. However, pintura's jsgi module, which uses the same require, loads ./node_modules/pintura/node_modules/perstore/errors.js . This ends up causing the instanceof operator to fail.

For now I've worked around this issue by changing the check to be:
if (e instanceof AccessError || (e.name && e.name=="AccessError"))

Which seems to do the trick for me for now, but perhaps there is a better way to fix this issue?

@kriszyp
Copy link
Member

kriszyp commented Feb 5, 2013

Yes, if there multiple perstore/errors, than the instanceof checks will definitely fail. My intent was that the persvr dependencies would be installed as the dependencies of a single persvr app so that perstore and pintura would be siblings and not need to have the isolated child dependencies that occurs when you install each of these packages individually.

If you don't do that, your fix does look correct, and I don't mind updating the code for that since it is likely that others would install the packages this way.

@dmachi
Copy link
Contributor Author

dmachi commented Feb 6, 2013

Thanks Kris. I don't think my way of setting up the app is unique. Its the same as the persevere-example-wiki. The issue is that when you define your app's dependencies in package.json, it downloads all of the deps you define in as siblings as you'd expect. However, where each of those modules has dependencies on each other, an additional copy (or at least a link) is made. So when pintura needs something from perstore, it looks in its own ./node_modules/ instead of ../node_modules/ Of course you could not list perstore as a dependency at the top of your application, but this doesn't work in regards to extending Facet/Model/ or using any of the interior utilities. I'm not aware of anyway to avoid this aside from perhaps using a different loader/require mechanism.

The problem with my suggested approach of using the name is that it gets messed up/confusing when there are extensions to an Error too. The name approach is simple enough and will do for now I think, though perhaps it needs to be a generated unique id, and each extension of that class automatically pushes itself onto the list/id of its parent. The jsgi error module could then be used for extended Errros too.

@kriszyp
Copy link
Member

kriszyp commented Feb 6, 2013

Another possibility would be to create a global error constructor (namespaced perhaps), and the module could check for the existence of the global error constructor before creating a new one.

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

No branches or pull requests

2 participants