Skip to content
This repository has been archived by the owner on Jan 5, 2021. It is now read-only.

Broken for simple arrays #7

Open
ro-ka opened this issue Nov 23, 2015 · 6 comments
Open

Broken for simple arrays #7

ro-ka opened this issue Nov 23, 2015 · 6 comments

Comments

@ro-ka
Copy link
Contributor

ro-ka commented Nov 23, 2015

Passing in an address array breaks. There is one address geocoded but not returned.

Example usage:

geoBatch.geocode(addresses)
  .on('data', (data) => console.log('geocodeResult', data))
  .on('end', () => console.log('geocodeEnd'));

It works in version 1.1.0, but not in version 1.2.0.

@pmast
Copy link
Member

pmast commented Nov 24, 2015

Do you have more info, e.g. an example for addresses.

This works:

const addresses = [
  'Hamburg',
  'Berlin',
  'Juliusstraße 25, 22769 Hamburg'
];

let geoBatch = new GeoBatch();

geoBatch.geocode(addresses)
  .on('data', (data) => console.log('geocodeResult', data))
  .on('end', () => console.log('geocodeEnd'));

I get three results, all are correct.

@Caerostris
Copy link
Contributor

I've run into a similar issue. The problem was that I had a syntax error in the .on('data') handler.
This resulted in an exception being thrown which disappeared somewhere deep inside the promises and streams of geobatch. In other words: If an exception is thrown inside one of your handlers, the entire node process will crash without any kind of error message. It just quits. This is probably what happened to you, @ro-ka

@ro-ka
Copy link
Contributor Author

ro-ka commented Jun 28, 2016

I can’t remember, but I think @pmast and I discussed this. 😃 Not sure which array I used… Do you have test data to replicate, @Caerostris?

@Caerostris
Copy link
Contributor

Caerostris commented Jun 28, 2016

The following code works as it should. It prints both results, and 'finished' at the end:
https://tonicdev.com/574c415398265b1300bbfcfa/577234fa44e49613003f4f4a

In the following example, an error has been introduced in the .on('data') event handler.
https://tonicdev.com/574c415398265b1300bbfcfa/577233108dc3051200d9dce0
This code produces no output at all.

In this example, the order of the faulty line and the console.log statement has been flipped.
https://tonicdev.com/574c415398265b1300bbfcfa/5772340673eff31300709a3d
It produces two results. The first one is as expected.
The second one, however, has the input field set to the first query, and the error field set to the error message thrown in the event handler.

In neither of the two examples the .on('end') event fires, and thus neither of the two print 'finished'.
(edit: FYI, no need to replace the API key placeholder. Google seems to allow a limited number of requests from invalid keys)

@ro-ka
Copy link
Contributor Author

ro-ka commented Jun 29, 2016

Ah, thanks! So we need a fix for this. :)

@Caerostris
Copy link
Contributor

So, the problem is that exceptions inside the data event handler get swallowed by a Promise.
It seems as if the only way to throw an exception from inside a Promise is

setImmediate(() => {
  throw new Error('Some error');
});

I could open a PR which try/catches exceptions in the data event handler and setImmediate-throws them. Feels like a bit of a hack though.
Any thoughts?

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

No branches or pull requests

3 participants