search

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

Handle array of errors. #289

Closed richardpringle closed 8 years ago

richardpringle commented 8 years ago

Temporary fix for strongloop/loopback#2123

Connect to strongloop/loopback#2123

richardpringle commented 8 years ago

@bajtos, the test case here is a little weird, please advise.

bajtos commented 8 years ago

the test case here is a little weird, please advise.

It looks good to me. What do you find weird? It's possible I am missing something.

richardpringle commented 8 years ago

@bajtos I just thought there should be a more simple way to verify that the error is correct instead of nesting requests.

bajtos commented 8 years ago

I just thought there should be a more simple way to verify that the error is correct instead of nesting requests.

I suppose you could simply check that the second request returns an error where details contains two entries and each entry has expected message? Such test will check less facts than your current implementation, but I think it should suffice.

Anyways, the current test looks good to me.

bajtos commented 8 years ago

@richardpringle I found few nitpicks, see the comments above. The patch LGTM otherwise, no further review is necessary. Please back-port to 2.x too.

richardpringle commented 8 years ago

@bajtos, changes made but I don't quite know what's going on with these failed tests... There seem to be some unrelated error to my changes.

Otherwise this is ready to land.

richardpringle commented 8 years ago

@slnode please test

richardpringle commented 8 years ago

@bajtos, there are random dependency tests failing. I haven't looked at all of them but in connector-rest there are 3 tests failing only on node 0.10 on ubuntu and in lb-boot, linter is failing for node 5 on ubuntu and amazon. Linter is also failing on all platforms for lb-workspace.

I'll check to see if the master branch manages to pass CI but in the meantime, do you have any ideas?

richardpringle commented 8 years ago

This build hasn't passed in 6 months...

bajtos commented 8 years ago

LGTM.

As for the failing CI builds of dependencies, I hate to say it, but we should ignore those random failures and simply land this PR.