by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • 19:11

    alexjeffburke on master

    Bump unexpected-documentation-s… (compare)

  • 14:58

    alexjeffburke on master

    Move to evaldown powered doc si… Allow bootstrap-unexpected-mark… Move to evaldown port of unexpe… and 20 more (compare)

  • 14:58
    alexjeffburke closed #721
  • 14:36
    alexjeffburke synchronize #721
  • 14:36

    alexjeffburke on evaldown

    Update to released versions. (compare)

  • Jun 28 12:00
    alexjeffburke edited #721
  • Jun 28 11:24
    alexjeffburke commented #721
  • Jun 28 11:22
    alexjeffburke ready_for_review #721
  • Jun 27 21:15
    alexjeffburke synchronize #721
  • Jun 27 21:15

    alexjeffburke on evaldown

    Duplicate Person in addType doc… Replace update-examples binary … (compare)

  • Jun 27 20:45
    alexjeffburke commented #721
  • Jun 27 20:32
    alexjeffburke synchronize #721
  • Jun 27 20:32

    alexjeffburke on evaldown

    correct git versions (compare)

  • Jun 27 15:15
    alexjeffburke commented #721
  • Jun 27 15:07
    alexjeffburke commented #721
  • Jun 27 14:53
    papandreou commented #721
  • Jun 27 14:07
    alexjeffburke commented #721
  • Jun 27 13:29
    alexjeffburke synchronize #721
  • Jun 27 13:29

    alexjeffburke on evaldown

    tweak language (compare)

  • Jun 27 13:21

    depfu[bot] on update

    (compare)

Gustav Nikolaj
@gustavnikolaj
And for the record, I probably use more code of his than I know. :)
Gert Sønderby
@gertsonderby
That's a good point, honestly. I've not, generally, ended up supporting older node versions, so I've not encountered that specific problem myself. But I have seen that type of problem show up, all the same.
Alex J Burke
@alexjeffburke
@sunesimonsen @papandreou I'm trying to pick up work on evaldown to see if it can be finished enough to be adopted. On that front, two big things: first, I adapted (with slight changes) the source rewriting from the node core repl, things still aren't fully isolated but it catches more cases and paves the way to do it eventually. Second, I've taken onboard the critique that definitions within a single file should continue to exist to the end of it so things just flow i.e. I'm trying to remove that "nowrap" flag. I'll share more once I'm confident enough to release the changes
Sune Simonsen
@sunesimonsen
@alexjeffburke nice
Andreas Lind
@papandreou
@alexjeffburke, great, looking forward to it! 👀
Sune Simonsen
@sunesimonsen
@papandreou I had a bit of progress on @transformation, I added new package for integrating with Node streams https://github.com/sunesimonsen/transformation/blob/master/packages/stream/Readme.md it was kind of fun to build :-)
Andreas Lind
@papandreou
That looks like a nice bridge that allows you to pull in a lot of already built software!
Sune Simonsen
@sunesimonsen
Even if it is fun to build everything, it isn't really practical 😅
Andreas Lind
@papandreou
Tell me about it :)
Peter Müller
@Munter
That talk we had about fake timers the other day. I managed to set up a test that works by only overriding setTimeout:
    let clock: sinon.SinonFakeTimers;

    beforeEach(() => {
      clock = sinon.useFakeTimers({
        toFake: ["setTimeout"],
      });
    });

    afterEach(() => {
      clock.restore();
    });

    it("should retry authentication at a given interval from the server", async () => {
      httpception([
        {
          response: {
            statusCode: 429,
            headers: {
              "Retry-After": 30,
            },
          },
        },
        {
          response: {
            statusCode: 429,
            headers: {
              "Retry-After": 30,
            },
          },
        },
        authSuccess,
      ]);

      const fastClock = setInterval(() => clock.tick(30000), 10);

      await authenticate("accountId", "password");

      clearInterval(fastClock);
    });
Andreas Lind
@papandreou
I guess that supports the theory that u-mitm and httpception should try to avoid being affected by the fake timers taking setImmediate and process.nextTick hostage?
Peter Müller
@Munter
I would say so, yes. I think the only thing needed is assigning them to a local variable at initialization time at the top of the relevant files. I had to do that in a test locally as well where I need to have the request retry 5 times and then fail. Needed to advance the clock and then wait for a few real milliseconds for the next tick to happen
Andreas Lind
@papandreou
Yeah, I've fixed similar clashes before with that trick
Sounds right
Peter Müller
@Munter

I have this test that passes:

    it("should fail after 5 retries", async () => {
      httpception([
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
      ]);

      const clientPromise = authenticate("accountId", "password");

      clock.tickAsync(30000);
      await wait(10);
      clock.tickAsync(30000);
      await wait(10);
      clock.tickAsync(30000);
      await wait(10);
      clock.tickAsync(30000);
      await wait(10);
      clock.tickAsync(30000);
      await wait(10);

      return expect(
        clientPromise,
        "to be rejected with",
        "Response code 429 (Too Many Requests)"
      );
    });

But in the console I get UnhandledPromiseRejectionWarning:

(node:15093) UnhandledPromiseRejectionWarning: HTTPError: Response code 429 (Too Many Requests)
    at onResponse (/Users/pbm/git/smartling-migration-tools/upload-all-languages/node_modules/got/dist/source/as-promise/index.js:124:28)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:15093) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:15093) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:15093) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)

What's the most elegant way to avoid those warnings? Put a .catch(err => throw err) on there and switch the assertion to to throw?

Alex J Burke
@alexjeffburke
Hmmmm - that’s actually a little weird, if httpception was leaking a promise I think it’d show up as unhandled in afterEach
you might be able to get around this completely by using httpception in “promise factory” mode - I’ll write it up in a few when Im@back at my desk
Gustav Nikolaj
@gustavnikolaj
It's not httpception leaking anything, it's just that clientPromise is resolved before a listener is added :)
// $ cat test.js 
const delay = ms => new Promise(resolve => setTimeout(resolve, ms));

(async function () {
    const promise = new Promise((resolve, reject) => setTimeout(() => reject(new Error('bar')), 20));

    await delay(50);

    return promise;
})().then(
    () => console.log('Done'),
    (err) => console.log('Error:', err),
)
$ node test.js 
(node:4444) UnhandledPromiseRejectionWarning: Error: bar
    at Timeout.setTimeout [as _onTimeout] (/.../test.js:4:78)
    at ontimeout (timers.js:498:11)
    at tryOnTimeout (timers.js:323:5)
    at Timer.listOnTimeout (timers.js:290:5)
(node:4444) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:4444) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Error: Error: bar
    at Timeout.setTimeout [as _onTimeout] (/.../test.js:4:78)
    at ontimeout (timers.js:498:11)
    at tryOnTimeout (timers.js:323:5)
    at Timer.listOnTimeout (timers.js:290:5)
(node:4444) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
Peter Müller
@Munter
Hmm... Can I add the assertion right after I create the promise, and then tick the clock? I guess I would have to await the assertion promise at the end then
Gustav Nikolaj
@gustavnikolaj
Something like this could work
    it("should fail after 5 retries", async () => {
      httpception([
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
      ]);

      const wrappedPromise = new Promise((resolve, reject) => {
        let isResolved = false;
        const setResolvedAnd = fn => (...args) => {
            isResolved = true;
            fn(...args);
        };

        authenticate("accountId", "password").then(
          setResolvedAnd(resolve),
          setResolvedAnd(reject)
        );

        (async () => {
            while (!isResolved) {
              clock.tickAsync(30000);
              await wait(10);
            }
        })();
      });

      return expect(
        wrappedPromise,
        "to be rejected with",
        "Response code 429 (Too Many Requests)"
      );
    });
And it is guaranteed to confuse anyone with its mix of different ways of using promises :)
You have to attach listeners and then tick the clock. The then-cb on the expect promise is not called until the clientPromise is resolved, which it won't be, until you tick the clock.
Peter Müller
@Munter
This is true. I could possibly set that fast clock ticking up before I create my client promise. Then I could await it. But then I won't have control over the clock in the same way. I do want to also make sure that the retries are made in the exact increments, no more, no less
Gustav Nikolaj
@gustavnikolaj
Like this?
    it("should fail after 5 retries", async () => {
      httpception([
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
        http429_30Seconds,
      ]);

      setImmediate(async () => {
          clock.tickAsync(30000);
          await wait(10);
          clock.tickAsync(30000);
          await wait(10);
          clock.tickAsync(30000);
          await wait(10);
          clock.tickAsync(30000);
          await wait(10);
          clock.tickAsync(30000);
          await wait(10);
      });

      return expect(
        authenticate("accountId", "password"),
        "to be rejected with",
        "Response code 429 (Too Many Requests)"
      );
    });
Peter Müller
@Munter
That's not half bad actually. I'll try that
That seems to work
Gustav Nikolaj
@gustavnikolaj
I don't like fake timers ( @alexjeffburke can attest to that) - and this is a quite good example; if you have a bug in the above and it will actually take 6 tries to error out, now you have a test that will take ${yourTimeout}-seconds to error, and it will not have a descriptive error. I prefer testing the retry-unit more directly and fakeout the actual requests instead. Then you don't need to worry about time. :)
BTW, I know it's a matter of taste and prefs btw - it's not a judgement statement - I just thought it was a good example :)
gustavnikolaj @gustavnikolaj hates timers
Peter Müller
@Munter
It's got under the hood, so the retry logic is not directly accessible. I'm testing the behavior a user can expect.
Gustav Nikolaj
@gustavnikolaj
arh, right
Peter Müller
@Munter
Actually this specific test errors out very quickly because another http request hits httpception. The problem is of course that failing the afterEach hook stops the run
Gustav Nikolaj
@gustavnikolaj

The problem is of course that failing the afterEach hook stops the run

promise factory-mode to the rescue :D

Peter Müller
@Munter
???
Gustav Nikolaj
@gustavnikolaj
it("should fail after 5 retries", httpception([
    http429_30Seconds,
    http429_30Seconds,
    http429_30Seconds,
    http429_30Seconds,
    http429_30Seconds,
], async () => {
    setImmediate(async () => {
        clock.tickAsync(30000);
        await wait(10);
        clock.tickAsync(30000);
        await wait(10);
        clock.tickAsync(30000);
        await wait(10);
        clock.tickAsync(30000);
        await wait(10);
        clock.tickAsync(30000);
        await wait(10);
    });

    return expect(
        authenticate("accountId", "password"),
        "to be rejected with",
        "Response code 429 (Too Many Requests)"
    );
}));
Alternative interface to httpception, that uses a wrapped promise instead of afterEach hooks :)
Peter Müller
@Munter
TIL !!!
Gustav Nikolaj
@gustavnikolaj
I abandoned my crusade for that interface after failing to drum up excitement around it, but I think it's still in httpception and fetchception :)
It should work the same, but not rely on hooks, and you can just think of it as wrapping your it-function.
I think it's a much simpler and easier to understand mode - but I have accepted that I'm just a bit off on that. But at least it should be able to solve your immediate problem :)
Peter Müller
@Munter
Yeah, I can deduct from the interface what it has to do. Which makes it a fairly ok abstraction to begin with
Wel, it's not liek the status quo is a bit problem. If I fail the test I get an assertion error that describes very well what happened. It just doesn't run the rest of the tests
Gustav Nikolaj
@gustavnikolaj
Yeah, it's not the end of the world :)
btw, that setImmediate-trick in the last code example should totally be using an aliased setImmediate named glitchInTheMatrix.
Alex J Burke
@alexjeffburke
argh sorry! I got dragged into a meeting before I could write that up - but the chunk gustav posted is exactly what I had in mind. And yes, I can vouch for fake timer hatred :P
Peter Müller
@Munter
I'm looking at offline-github-changelog and trying to figure out how I could set up a github action that will automatically generate a github release message from it. Could it make sense to add a command line param to have it only output the latest version?