Back to Ideas 7 min read

One Way to Fail

E
EkoHacks Team
·
One Way to Fail

An upload should fail the same way everything else fails. For a while ours did not. A failed method already came back as { code, message }, but the upload route still had a reply or two in an older shape, and a test sat parked on that gap so it could not be quietly forgotten. This is the post where the gap closes.

It is worth saying what closing it actually took, because it was not more code. Most of the work that finished the smoke test, the one that proves you can upload a BAM file and watch it land, was deletion. An upload learned to fail the same way every other call fails, and the way it learned was by giving back things we had grown along the way. Here is EkoLite, our small real time backend, losing weight.

The server learns to say no

Before any of the client work, the route was indiscriminate. POST /api/files would write a .txt or a .zip to disk as happily as a .bam, because nothing looked at the name before saving. The first change was a gate, and it went where uploads already live, in Files, the class that saves the bytes and records the document.

The rule is a pure check, and the allowed set is configurable rather than a buried literal:

const ALLOWED_EXTENSIONS = ['bam'];

validate(name: string): boolean {
  return this.isAllowed(extensionOf(name));
}

The guard runs before the write, so a refused file never reaches disk or the database:

async upload(input: UploadInput): Promise<StoredFile> {
  const extension = extensionOf(input.name);
  if (!this.isAllowed(extension)) {
    throw fileUploadError(extension);
  }

  await this.storage.save(input.name, input.data);
  // build the document, insert it, return it
}

A new format later is a line of config, not a code change. And the server is the thing that actually refuses, so a bad file cannot land just because someone bypassed the UI.

The error we already had

Look at that throw fileUploadError(extension). The first cut of this gave the client its own upload error type, a bespoke shape that only the uploader knew about. It passed its tests. It was also a second error language in a codebase that already had one.

So we deleted it. fileUploadError is now a one line helper over the error we already had:

export function fileUploadError(extension: string): RpcError {
  return new RpcError(400, `Unsupported file type: .${extension}`);
}

RpcError is the same class a failed method call rejects with. The route does not special case it. A single error handler turns any thrown error into the shared envelope and sends it back at the right status:

server.setErrorHandler((err, _request, reply) => {
  const error = toEkoLiteError(err);
  reply.status(error.code).send(error);
});

That is the whole convergence. The socket path and the HTTP path now refuse in one shape, { code, message }, and the test we parked on the gap turned green not because we wrote something but because we removed the thing that made the shapes differ. One error language. The client, when we get to it, rejects with the same RpcError its method caller already rejects with, so a caller handles an upload failure and a method failure the same way.

A transport you can switch off

The client uploader copied a move the socket made long ago. upload() never news up an XMLHttpRequest of its own, because the moment it does it stops being testable and drags the browser back into a unit test. Instead it takes a transport the way the socket takes a WebSocketFactory:

static create(): Uploader {
  return new Uploader(() => new RealRequest());
}

static createNull(options: NullUploaderOptions): Uploader {
  return new Uploader(() => new NullRequest(options.response));
}

RealRequest delegates to the browser. NullRequest simulates a response. Your code runs the same either way, which is the entire point of the off switch.

There is a small evolution worth naming here, because it is the kind of thing the drawing never gets right. The plan for this named the seam XhrLike, with a RealXhr and a NullXhr. The code settled on RequestLike, RealRequest and NullRequest. The sketch was right about where the seam went and wrong about what to call it, which is the usual split. A plan names the seam. The code names the types, once it can feel which name reads.

And the transport is XMLHttpRequest rather than fetch, even though fetch would make the happy path shorter, because a progress bar is the next slice and fetch cannot report upload progress. We put the seam where the future need already is, so the next story hangs a bar off the same request instead of rewriting it.

The bug the seam let us keep small

The first version of the uploader held a single request and reused it. That looks fine until two uploads run at once, or one runs after another, and they fight over a request that has already been spent. The fix was one word in the design: hold a factory, not an instance, and mint a fresh request per call. A test that starts two uploads together and expects both to finish pins it so it cannot come back. The seam is what made the bug a one line correction rather than a rewrite.

One place to decide

The last refinement was to keep the resolve or reject decision in one place, reading the status once, so there is a single answer to whether an upload succeeded:

private resolveUpload(request: RequestLike): UploadResponse {
  const parsed: unknown = JSON.parse(request.responseText);

  if (request.status >= 200 && request.status < 300) {
    if (!isUploadResponse(parsed)) {
      throw new Error('Invalid upload response');
    }
    return parsed;
  }

  if (!isUploadError(parsed)) {
    throw new Error('Invalid upload error');
  }
  throw new RpcError(parsed.code, parsed.message);
}

A 2xx resolves with { id, name }. Anything else rejects with the shared error. A 2xx that comes back the wrong shape rejects rather than hanging, so every path settles, one way or the other, and there is one method to read when you want to know what an upload does.

Two paths become one

The demo had been proving the round trip with a throwaway fetch, written before the uploader existed. With a real uploader in the tree, keeping the fetch meant two upload paths, and only one of them was the one we ship. So the demo lost its fetch and gained the real surface:

const uploader = Uploader.create();
// ...
void uploader
  .upload(file)
  .then((stored) => log(`in: stored ${stored.name} (${stored.id})`))
  .catch((err: unknown) => {
    if (err instanceof RpcError) {
      log(`upload refused: ${String(err.code)} ${err.message}`);
    }
  });

The demo now runs the exact code production runs. A refused .txt used to log a bare upload failed: 400. Now it shows the server's own words, upload refused: 400 Unsupported file type: .txt, because the refusal carries the message the whole way out.

The move you can reproduce

You do not have to take our word for the off switch, which is the part that matters. A refused upload is testable with no network and no browser. Configure the null uploader with the response a real server would send, and assert the rejection:

const uploader = Uploader.createNull({
  response: { status: 400, body: { code: 400, message: 'Unsupported file type: .txt' } },
});

await expect(
  uploader.upload(new File([Buffer.from('notes')], 'bad.txt')),
).rejects.toMatchObject({ code: 400, message: 'Unsupported file type: .txt' });

That test reaches nothing. It runs the real upload logic against a stub of the transport, and it proves the refusal arrives as the shared error. Swap createNull for create and the identical code talks to a live server. One line is the difference between the test and production, which is the only honest way to know the test was watching the real thing.

Why we work this way

The shape we never drew is finished now, and finishing it was mostly giving things back. One bespoke error type, gone. One spare upload path, gone. One guess about where the collection lived, gone earlier. What is left is smaller than what we started with and says more.

That is the trade emergent design keeps offering, if you let it. You grow a thing to feel its shape, and then, when the shape is clear, you delete your way down to it. The discipline that makes the deletion safe is the same one as always: every piece integrated all the time, a test on every claim, and a deep suspicion of any code that only your own tests seem to need. An upload should fail the same way everything else fails. Now it does.

E

Written by

EkoHacks Team

More from Ideas

·6 min read

What the Nullable Gave Back

One file, seven behaviours held fixed, the database swapped for a Nullable: about 180 times less time inside the tests, and coverage flat to two decimals.

E
EkoHacks Team
·6 min read

Twenty Six More Tests, Four Fewer Behaviours

Removing the mocks grew the suite from 44 tests to 70 and quietly deleted four behaviours, every one of them a failure path. Test count is not coverage.

E
EkoHacks Team
·6 min read

The Best Coverage Number in the Room

Same commit, same spec, same test count. The mocked suite ran 5.6 times faster, covered 3.5 fewer points of real code, and posted the best branch coverage.

E
EkoHacks Team