Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adapter errors #376

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
Member

tchak commented Sep 6, 2012

In this PR I extracted only basic stuff from #360

It should be more readable

Member

ghempton commented Sep 6, 2012

+1

@tchak tchak DS.AdapterError
The error state is now used only for fatal errors.

Validation errors deserve they own state, because validation error has something
to do with actual data.

Other type of errors have no effect on data. They just say "I can not save now, try later"

This is why, non fatal errors, act as a decorator, independent of record state.
273aec7

@chrisdurtschi chrisdurtschi commented on the diff Sep 14, 2012

packages/ember-data/lib/adapters/rest_adapter.js
@@ -306,6 +318,44 @@ DS.RESTAdapter = DS.Adapter.extend({
}
},
+ handleError: function(store, records, jqXHR) {
+ var responseHash = this.parseResponseText(jqXHR);
+
+ if (jqXHR.status === 422) {
+ records.forEach(function(record) {
+ var errors = this.materializeValidationErrors(record, responseHash['errors']);
+
+ store.recordWasInvalid(record, errors);
+ });
@chrisdurtschi

chrisdurtschi Sep 14, 2012

I think you need to pass this in as the context here

@mars mars commented on the diff Sep 21, 2012

packages/ember-data/lib/system/model/states.js
var dirtyType = get(this, 'dirtyType'),
record = get(manager, 'record');
+ get(record, 'errors').add(errors);
+
record.withTransaction(function (t) {
t.recordBecameInFlight(dirtyType, record);
@mars

mars Sep 21, 2012

If the record's state is "inflight", then we can't do anything useful with it, like rollback() or commit().

Shouldn't a record with a 422 response from the server transition to "uncommitted"?

@wildchild

wildchild Feb 17, 2013

@mars, +1. Why it's in flight after unsuccessful server validation?

+1

This fails when deleteRecord returns error state, it throws following error
DS.StateManager:ember1453> could not respond to event becameInvalid in state rootState.deleted.inFlight.

What's the reasoning behind not handling adapter errors on GET requests?

Owner

wagenet commented Oct 17, 2012

@tchak Looks like the build is failing. Can you look into this?

stwr667 commented Oct 19, 2012

+1

flexd commented Nov 15, 2012

+1

svasva commented Nov 15, 2012

+1

Owner

wagenet commented Nov 15, 2012

+1's do nothing

Member

tchak commented Nov 15, 2012

This is not useful as is, I will reopen it backed by a proper proposition

@tchak tchak closed this Nov 15, 2012

Contributor

enyo commented Mar 25, 2013

@wagenet This is some random issue I just found and stumbled upon your +1 pic. Although it made me giggle, isn't it the only way to express desire for a given feature at the moment?
I think that Github issues really miss a built in +1 feature so authors know which features are desired the most, but until then I always felt that commenting +1 is the next best thing. I for one always like knowing which features are needed and requested the most to prioritise my issues.

Owner

wagenet commented Mar 25, 2013

@enyo The main thing is that we don't really use Github issues to get a track on what people want. In many cases, the core developers already have their own internal roadmap (which we should do a better job of publicizing). Either the things people want fit into that roadmap and will get addressed accordingly, or they are actually not as important to the core developers and so won't be prioritized by them. In either case, the core developers have a pretty big queue of work in front of them and aren't even close to the general stage of discretionary prioritization to the point where voting has an effect.

That said, if users have a very specific problem or need that is addressed by a proposed feature, it is useful for them to provide more background. In most cases we already know that people want a feature, but sometimes the additional background can provide information that we weren't already aware of.

In this specific case, the core team already knows that adapter errors are a very important feature, but they are just one of many very important features. They will be addressed as soon as is possible.

Contributor

enyo commented Mar 25, 2013

@wagenet you're right that +1ing a bug seems rather unnecessary since obviously bugs are always kind of a priority for developers. When it's about features though it can be useful to know which ones should be prioritised.

I think it really depends on the project (and the developers) if it can be useful or not. I just wondered why you seemed to be bothered by it, when I find it rather useful in my projects (which I maintain in my free time). I think that people mean well and want to show their enthusiasm and eagerness for certain features.

Anyway... thanks for your work on ember!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment