I feel like we shouldn’t deprecate a core feature just because the guide recommends a different approach, but PRs to the guide to make it more consistent would be much appreciated. Perhaps we can also add a note to the docs in the check section which says you might prefer this other approach.
This sounds like a good idea. I actually like the idea of ValidatedError handling because it tends to send along a details array, which is great for dealing with errors in the client UI.
I guess I just wonder about the disconnect that may happen if someone looks at the docs and not the guide? Perhaps just a mention/link to a guide article is enough. Perhaps this discussion points to the need for a Validation article or expanded subsection in the security guide (point 1 is the only mention here: http://guide.meteor.com/security.html#method-rules)?
You can even see the inconsistencies in the security portion of the guide. It took me a good 1-2 hours of playing around to understand the upside of using SimpleSchema. I also worry about the migration to NPM and what does that mean for a package (SimpleSchema) that seems to have a last commit of December 2015? Does this mean MDG would be willing to own update/maintenance releases of the package since it is recommended now? Perhaps @aldeed can chime in on long term goals?
2 Likes
export default updateText = { ...
Is this code working?
I followed the ‘Code Style’ instruction, but ESLint pointed it out as an error.
As it stands it isn’t valid when linted because updateText won’t be defined. To fix it you could change it to something like:
export const updateText = { ...
I have a method to parse an xlsx file. It’s a ‘server-only’ function, what’s the best approach here?
This is my current setup:
/objects/methods.objects.import.js
import xlsx from 'xlsx'; // I don't want to bundle this to the client...
let import = {
name: 'objects.import',
validate(args) {...},
run({..}) {...},
call(args, callback) { ... }
};
export default import;
Meteor.methods({
[import.name]: function (args) {
import.validate.call(this, args);
import.run.call(this, args);
}
});
Right now I have a methods.objects.import.server.js and a methods.objects.import.client.js (with an empty implementation). But this seems kind of clumsy, no?
Any ideas?
@joshowens I am doing a final push toward an RC for SimpleSchema 2.0, which is an NPM package. It fixes all known issues with current SimpleSchema Meteor package and adds some requested features, but also has a few compatibility breaks. It’s done, but I am rewriting tests and doing a bit of refactoring still.
I will need to work with MDG to port ValidatedError class to an NPM package, unless someone wants to tackle that for me. Somewhat problematic is how it wraps Meteor.Error, which would also have to be moved to NPM. My preference would be for the DDP packages to forward any Error based on the presence of some boolean property like clientSafe: true rather than on being an instance of Meteor.Error.
5 Likes
I am soooo glad to hear that!
4 Likes
I took a brief look at Meteor.Error and was a little surprised to find that the “Meteor” package that defines the “Meteor” global only appears to have one dependency on anything not already ported to NPM (according to the package.js). Maybe I’m missing something and the package is somehow rife with global variable references. But, if not, it might not be so crazy to ask MDG to push the port of that particular package to NPM into release 1.4 instead of 1.5. The 1.4 release may align well with your timetable.
Of course, your preferred solution of changing the forwarding of errors would probably allow your package to serve a more general audience.
Yeah I like @aldeed solution of using duck typing rather than checking for Meteor.Error specifically.
@aldeed I’m super-excited about SimpleSchema 2.0 and happy to try the ValidationError NPM port if that helps move things along faster.
@sashko It seems like Meteor.Error defines a field errorType whose value is always the string “Meteor.Error”, so any code that checks insanceof Meteor.Error could safely check `result.errorType === “Meteor.Error” instead.
If you guys are good with that solution, I can try to put together PRs for meteor/validation-error and meteor/meteor that implements this. Or if you have other suggestions, I can try that as well. Please let me know what you think!
A thought for all… when I looked at this I was thinking that instead of changing meteor/validation-error, SimpleSchema 2.0 could have a configuration that could be called up front to supply a function for building and raising errors that accepts the same fields as ValidationError. We could then just supply whatever is desired at runtime. That would be more flexible in terms of supporting environments other than meteor.
2 Likes
Either one of those sounds good to me!
@shilman “Meteor.Error” is a misnomer in the first place because it only exists for DDP as far as I can tell, so it should be called “DDP.Error” or “DDP.ClientError”. That said, I’d prefer to generalize more than that because I think the idea of a “standards spec” for throwing client-safe errors would be useful for many different frameworks beyond Meteor. I think the simplest and most generic way to “spec” this idea is a boolean prop clientSafe: true on any Error instance that is thrown.
My proposal for a standard spec would be something like this:
“When an Error is thrown while executing code as part of an HTTP or Websocket request, catch and examine the Error instance. If error.clientSafe is exactly true, then error.name, error.message, error.error, error.reason, and error.details may all be assumed to be safe to include in the response sent to the requesting client. Alternatively, if error.sanitizedError is an Error instance, then error.sanitizedError.name, error.sanitizedError.message, error.sanitizedError.error, error.sanitizedError.reason, and error.sanitizedError.details may all be assumed to be safe to include in the response sent to the requesting client. The error stack should never be sent.”
To follow this spec within Meteor/DDP, I think we only need to add an || error.clientSafe === true to the condition, and change the sanitizedError check to work when sanitizedError is any type of Error and not just Meteor.Error.
Thoughts, @sashko?
2 Likes
Also, @rlivingston we can add a custom error function, too, since that will likely be necessary in the short term until everyone can get onto a Meteor version where things work out of the box.
2 Likes
That makes sense. I’ve documented your proposal as a github issue, and am happy to make the code changes, add unit tests, update documentation, and whatever else is required to make this happen:
@aldeed @sashko please advise and feel free to comment on the issue, assign to me, etc. or I can create a PR and we continue the conversation there.
There’s still no mention of promises in guide other than in context of npm packages, is the expectation then to introduce dependencies in face of a solid choice (official) or just use wrapasync which apparently is a package for callback hell.
Most of the promise based packages are deprecated, citing introduced support for promises officially.
You can now use async/await directly in Meteor.
but wheres the documentation. as in how make method be promise.
async function asdf () {
var a = await Meteor.call("refreshToken", Meteor.user())
console.log(a)
return a
}
This for example doesn’t appear to do anything on client side.
1 Like