Alias NextMsg to Fetch for Pull Subscribers

[Jarema]

Addresses #747

[coveralls]

Coverage decreased (-0.03%) to 86.804% when pulling dad4664 on Jarema:master into cc4f3b2 on nats-io:master.

[Jarema]

I tried to make changes in the test as little and not-confusing as possible. Let me know if it's good enough.

[derekcollison]

Thanks will take a look!

[wallyqs]

Based on user feedback and issues such as #785 it seems that might help to make NextMsg() become an alias to Fetch(1), wdyt? @ColinSullivan1 @kozlovic @scottf

[kozlovic]

@wallyqs Except that with Fetch() you could pass options that you can't with NextMsg(). Maybe it is better to fail for pull subscriptions (what I do in C), to make it clear that this is not a regular sync subscription. But I could be convinced otherwise.

[wallyqs]

But the options for Fetch are only timeout related (MaxWait for a timeout or a Context) so might be fine, though as you mention it could be that we should just fail in case NextMsg is attempted to be used instead of just timing out.

[kozlovic]

@wallyqs Well, in the C client, and maybe the Go client if we update the implementation, the pull subscription will use a sync subscription, so in a way I prevent the user from calling NextMsg(), since i use that internally by sending MSG.NEXT request with the reply subject of the sync sub was created on. The difference is that Fetch() is going to "filter out" the status messages possibly sent by the server.

Now, I realize that I am mudding things up: if the intent is to have NextMsg() for a pull subscribe to basically do:

   msgs, err := sub.Fetch(1)
   if err != nil {
      return nil, err
   }
   return msgs[0], nil

then I guess I am fine with that.

[scottf]

I'm against conflating NextMsg and Fetch. Also, Fetch is a "macro" that sets up a pull and does the NextMsg for the user. All 3 varieties of pull require the user to call NextMsg. It seems to me that this concept is getting lost

[kszafran]

I just spend some time myself scratching my head trying to figure out why NextMsg didn't read any messages (I was using a pull subscriber).

How about instead of aliasing it to Fetch we panic with a message like "use Fetch instead of NextMsg to fetch messages with a pull consumer"? It will also solve the problem with confused newcomers (like me), but it'll make sure that there's only one way to fetch messages with a pull subscriber, not two. IMO aliasing to Fetch does not solve anything that panic wouldn't solve, but it might make code maintenance more difficult in the future (for both the library maintainers and library users).

[kozlovic]

I apologize for the delay but we have decided to make calling NextMsg() on a pull subscription an error (you need to call Fetch). The error will be ErrTypeSubscription. This will be part of an upcoming PR since we have lot of planned changes for JetStream. The PR should be up today/tomorrow or Monday at the latest. So I am closing this PR.